Skip to content

Test-Json: use JsonSchema.Net instead of NJsonSchema #18023

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gregsdennis wants to merge 26 commits into from
Closed

Test-Json: use JsonSchema.Net instead of NJsonSchema #18023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
c774a81
updated Test-Json cmdlet to use JsonSchema.Net instead of NJsonSchema
gregsdennis Sep 3, 2022
a09c2c5
change quoting of test-json tests
gregsdennis Sep 5, 2022
fd12baf
rebuild files.wxs
gregsdennis Sep 5, 2022
f485dd4
adjust brace spacing to match examples in other commands
gregsdennis Sep 6, 2022
41dcbc9
remove space between closing curly brace and closing parenthesis
gregsdennis Sep 6, 2022
6858f50
configure for automatic downloads of external schemas
gregsdennis Sep 7, 2022
527aab0
use single quote strings when writing literal JSON values
gregsdennis Sep 7, 2022
ff7384e
Update test/powershell/Modules/Microsoft.PowerShell.Utility/Test-Json…
iSazonov Sep 8, 2022
7047a32
detect validation failures and report errors better
gregsdennis Sep 8, 2022
e98f6ce
remove pdb files from files.wxs
gregsdennis Sep 9, 2022
dbcdfd9
capture deserialization exception and record as failure to read ref'd…
gregsdennis Sep 9, 2022
086d0ed
remove component ids for pdb files from .wxs file
gregsdennis Sep 9, 2022
defd150
remove unused usings; add comment regarding json exception during val…
gregsdennis Sep 10, 2022
cfae3ac
add schema resolution exception; wrap fetch exceptions in new excepti…
gregsdennis Sep 11, 2022
fe74576
use WriteError instead of throwing exception
gregsdennis Sep 11, 2022
e9c179c
fix xml comments
gregsdennis Sep 11, 2022
80a21b9
Update src/Microsoft.PowerShell.Commands.Utility/commands/utility/Jso…
gregsdennis Sep 12, 2022
7788433
Update src/Microsoft.PowerShell.Commands.Utility/commands/utility/Tes…
gregsdennis Sep 12, 2022
eb6d1d0
use single-line method call; remove unused usings
gregsdennis Sep 12, 2022
af3603c
enable nullable refs in JsonSchemaReferenceResolutionException.cs
gregsdennis Sep 17, 2022
fc66799
add removed files from wxs
gregsdennis Sep 18, 2022
d7fbd12
add more removed files from wxs
gregsdennis Sep 18, 2022
d56dd1f
remove njsonschema from wxs
gregsdennis Sep 18, 2022
b63ab7d
regenerate files.wxs
gregsdennis Sep 19, 2022
d7502c2
set files.wxs to match local build
gregsdennis Sep 20, 2022
c4d860a
Fix files.wxs
iSazonov Sep 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 30 additions & 22 deletions assets/wix/files.wxs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@
<Component Id="cmp31176C5A3BDB4A9F95171A4780D7D4D2">
<File Id="fil31176C5A3BDB4A9F95171A4780D7D4D2" KeyPath="yes" Source="$(var.ProductSourcePath)\System.ServiceModel.Syndication.dll" />
</Component>
<Component Id="cmp3CC027D3F160412C9F0044EBED3115DD">
<File Id="filFD2EF6BC74AF459D1BB52CA1E8C6E33B" KeyPath="yes" Source="$(var.ProductSourcePath)\NJsonSchema.dll" />
</Component>
<Component Id="cmp3B130879A26D2E954251BB81E8948069">
<File Id="filAACDEEE28FEA076C73D082A0AD21B8E0" KeyPath="yes" Source="$(var.ProductSourcePath)\Microsoft.CSharp.dll" />
</Component>
Expand Down Expand Up @@ -1587,17 +1584,17 @@
<Component Id="cmp7C72179331DD48BCB12BF205F34B76AD">
<File Id="filF65735DC9B8B4957B27F5B33D2A322F4" KeyPath="yes" Source="$(var.ProductSourcePath)\ref\System.Runtime.InteropServices.JavaScript.dll" />
</Component>
<Component Id="cmpF3EB02BB62A34F509B22CC370C430A20">
<File Id="fil489339762CDF41AE97538C020401BEC6" KeyPath="yes" Source="$(var.ProductSourcePath)\ref\System.Net.Quic.dll" />
<Component Id="cmpBB2921B987554B68B26862747C3BD398">
<File Id="fil1D77B59F22194840A66EFA782AB2BB95" KeyPath="yes" Source="$(var.ProductSourcePath)\ref\System.Text.Encodings.Web.dll" />
</Component>
<Component Id="cmp2BB1C07A912442B0BB6270D7EB3185F3">
<File Id="fil2D6E6E560F1743729D90C204D69C369C" KeyPath="yes" Source="$(var.ProductSourcePath)\ref\System.Formats.Asn1.dll" />
<Component Id="cmpD182C2F62588471F82252B0038C75327">
<File Id="filBD7A930797F148DA8F433C809DC7BFB4" KeyPath="yes" Source="$(var.ProductSourcePath)\ref\System.Formats.Asn1.dll" />
</Component>
<Component Id="cmpF5FD2E392BD94BCD957480F128A38E10">
<File Id="fil9C11C1DAFDC94C0588965EA6EAAA7592" KeyPath="yes" Source="$(var.ProductSourcePath)\ref\System.Text.Encoding.CodePages.dll" />
<Component Id="cmpF3EB02BB62A34F509B22CC370C430A20">
<File Id="fil489339762CDF41AE97538C020401BEC6" KeyPath="yes" Source="$(var.ProductSourcePath)\ref\System.Net.Quic.dll" />
</Component>
<Component Id="cmp28DEAACC47FD4B04A95521A375761643">
<File Id="filE0DD0282CF6C440C99209B0B7495422C" KeyPath="yes" Source="$(var.ProductSourcePath)\ref\System.Text.Encodings.Web.dll" />
<Component Id="cmp16FC1857F01A4E388616BA7AA8CC5F73">
<File Id="fil877D1292F59541DAB6758F5A7A74D20D" KeyPath="yes" Source="$(var.ProductSourcePath)\ref\System.Text.Encoding.CodePages.dll" />
</Component>
</Directory>
<Directory Id="dirC72EEAF9434D4AD066E0F4D20D8E816C" Name="en-US">
Expand Down Expand Up @@ -2921,9 +2918,6 @@
<Component Id="cmp41E9A7E0470540228A69F13F8585089E">
<File Id="fil4F7CB5570E86407C94595812662A4F42" KeyPath="yes" Source="$(var.ProductSourcePath)\Microsoft.VisualBasic.Core.dll" />
</Component>
<Component Id="cmp8E8AE88416D44A4CA39F23E9C825A899">
<File Id="fil4954FEF58A894EA7B701DFCA7D11AF08" KeyPath="yes" Source="$(var.ProductSourcePath)\Namotion.Reflection.dll" />
</Component>
<Component Id="cmpE5F62B056F174A3E8843DFE8B70E3AA9">
<File Id="fil6B276B69CAC64F19A2A3E2A72086EF15" KeyPath="yes" Source="$(var.ProductSourcePath)\Markdig.Signed.dll" />
</Component>
Expand Down Expand Up @@ -3150,8 +3144,20 @@
<Component Id="cmp42A739F2AAC24D729DDBCAA096B8C55A">
<File Id="fil48314E2AC49B4E208C5667CEC50CA78C" KeyPath="yes" Source="$(var.ProductSourcePath)\RegisterMicrosoftUpdate.ps1" />
</Component>
<Component Id="cmp76EB2749D0464D41AD1D2BA2C51CF4C9">
<File Id="filDC81A93ACA6045E4A22C108F59D69083" KeyPath="yes" Source="$(var.ProductSourcePath)\mscordaccore_$(var.FileArchitecture)_$(var.FileArchitecture)_7.0.22.42610.dll" />
<Component Id="cmp5B1AAAB6B46D45EA8A430A766325B0D5">
<File Id="filF4983633E7914A1AAD5D02BFDFA7DD63" KeyPath="yes" Source="$(var.ProductSourcePath)\JsonSchema.Net.dll" />
</Component>
<Component Id="cmp61D33FB49B2E4C7D8E9C5EFC3F3A04B2">
<File Id="fil78E3FEFE7B0E4FE1993AC9C117B582C5" KeyPath="yes" Source="$(var.ProductSourcePath)\JetBrains.Annotations.dll" />
Copy link
Collaborator

@iSazonov iSazonov Sep 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder we have to include the dependency.
I think that's the problem. I don't think that's what we want to distribute.

And what is official license for the file?

Copy link
Contributor Author

@gregsdennis gregsdennis Sep 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.nuget.org/packages/JetBrains.Annotations/

License is MIT.

I've had requests to remove this before. It's actually a dependency. I use [RegexPattern] in a few of my methods where a Regex string is expected. It provides Regex syntax highlighting in the string literal. If there's a way to get this without the runtime dependency, I'm unaware of it.

Copy link
Collaborator

@iSazonov iSazonov Sep 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It provides Regex syntax highlighting in the string literal.

You could use .Net attribute https://source.dot.net/#System.Text.RegularExpressions/System/Text/RegularExpressions/Regex.cs,59

Copy link
Contributor Author

@gregsdennis gregsdennis Sep 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will require an update to the library, so I'll have to update the version ref in the .csproj file after I publish. Shouldn't take long.

Copy link
Contributor Author

@gregsdennis gregsdennis Sep 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That attribute is only available in .Net 7 preview. My library is .Net Standard 2.0. I can't use this.

Copy link
Collaborator

@iSazonov iSazonov Sep 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From docs:

By default, all annotation attribute classes are marked with [Conditional("JETBRAINS_ANNOTATIONS")] attribute so the compiler will ignore the attribute usages in your code, which means that no binary reference to the 'JetBrains.Annotations.dll' assembly will be produced. However, you can define 'JETBRAINS_ANNOTATIONS' conditional compilation symbol in your projects to preserve the attributes in the metadata.

So you can remove the dependency from binary package. Or You can also embed attribute declarations

Copy link
Contributor Author

@gregsdennis gregsdennis Sep 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That removes the attribute from the compilation. That doesn't help my users, which is the entire point of including the attribute. I don't need it for my own development; it's so that the people who use the library get the benefit.

Copy link
Collaborator

@iSazonov iSazonov Sep 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use embed attribute declarations

</Component>
<Component Id="cmpB51390A44A2E4D2EB27C891E4ED3A705">
<File Id="fil69092C4DDAE14264A0D1E788993DB43D" KeyPath="yes" Source="$(var.ProductSourcePath)\Json.More.dll" />
</Component>
<Component Id="cmpB5F59B2105D54C0D896C8D7EB3EA82F3">
<File Id="filB6F08AEE602D4982A38EB3532F62BCF3" KeyPath="yes" Source="$(var.ProductSourcePath)\JsonPointer.Net.dll" />
Copy link
Collaborator

@iSazonov iSazonov Sep 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you put all in one JsonSchema.Net.dll?

Copy link
Contributor Author

@gregsdennis gregsdennis Sep 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because JsonPointer.Net is used by other libraries in my json-everything suite: https://github.com/gregsdennis/json-everything

</Component>
<Component Id="cmp9ADAB04764E14F0DA83A54E4CE28319F">
<File Id="fil58C3F617C0DE48C0895D80C0FA26CCAB" KeyPath="yes" Source="$(var.ProductSourcePath)\mscordaccore_$(var.FileArchitecture)_$(var.FileArchitecture)_7.0.22.42610.dll" />
</Component>
</DirectoryRef>
</Fragment>
Expand All @@ -3175,7 +3181,6 @@
<ComponentRef Id="cmp4477FC97930D452DA018C8F6A3D7D168" />
<ComponentRef Id="cmp30C10EC477714A76B2A8D0A957D1D6F3" />
<ComponentRef Id="cmp31176C5A3BDB4A9F95171A4780D7D4D2" />
<ComponentRef Id="cmp3CC027D3F160412C9F0044EBED3115DD" />
<ComponentRef Id="cmp3B130879A26D2E954251BB81E8948069" />
<ComponentRef Id="cmp02ABBE4A3EDBEBFD05DC17A009A2B79D" />
<ComponentRef Id="cmpAA10498DF244C013CB5043C62E3AA83A" />
Expand Down Expand Up @@ -4047,7 +4052,6 @@
<ComponentRef Id="cmpFF8643F8CC97462CAC20815F6071203B" />
<ComponentRef Id="cmp0FC1F6198C66492495785315CDEE0B66" />
<ComponentRef Id="cmp41E9A7E0470540228A69F13F8585089E" />
<ComponentRef Id="cmp8E8AE88416D44A4CA39F23E9C825A899" />
<ComponentRef Id="cmpE5F62B056F174A3E8843DFE8B70E3AA9" />
<ComponentRef Id="cmp30126C60BD784974B74EA45AE9B04032" />
<ComponentRef Id="cmp9EC1507780D247E095EA92A7715B4BAC" />
Expand Down Expand Up @@ -4159,13 +4163,17 @@
<ComponentRef Id="cmp97D2E588E938462194C7549B3A0084FE" />
<ComponentRef Id="cmp41C949169E9341CC9297641456FEE2CC" />
<ComponentRef Id="cmp7C72179331DD48BCB12BF205F34B76AD" />
<ComponentRef Id="cmpBB2921B987554B68B26862747C3BD398" />
<ComponentRef Id="cmp0721DF55B34849B78C98AE00D863CBFF" />
<ComponentRef Id="cmpD182C2F62588471F82252B0038C75327" />
<ComponentRef Id="cmpF3EB02BB62A34F509B22CC370C430A20" />
<ComponentRef Id="cmp16FC1857F01A4E388616BA7AA8CC5F73" />
<ComponentRef Id="cmp42A739F2AAC24D729DDBCAA096B8C55A" />
<ComponentRef Id="cmp2BB1C07A912442B0BB6270D7EB3185F3" />
<ComponentRef Id="cmp76EB2749D0464D41AD1D2BA2C51CF4C9" />
<ComponentRef Id="cmpF5FD2E392BD94BCD957480F128A38E10" />
<ComponentRef Id="cmp28DEAACC47FD4B04A95521A375761643" />
<ComponentRef Id="cmp5B1AAAB6B46D45EA8A430A766325B0D5" />
<ComponentRef Id="cmp61D33FB49B2E4C7D8E9C5EFC3F3A04B2" />
<ComponentRef Id="cmpB51390A44A2E4D2EB27C891E4ED3A705" />
<ComponentRef Id="cmpB5F59B2105D54C0D896C8D7EB3EA82F3" />
<ComponentRef Id="cmp9ADAB04764E14F0DA83A54E4CE28319F" />
</ComponentGroup>
</Fragment>
</Wix>
2 changes: 1 addition & 1 deletion src/Microsoft.PowerShell.Commands.Utility/Microsoft.PowerShell.Commands.Utility.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.4.0-1.final" />
<PackageReference Include="System.Threading.AccessControl" Version="7.0.0-rc.1.22426.10" />
<PackageReference Include="System.Drawing.Common" Version="7.0.0-rc.1.22426.10" />
<PackageReference Include="NJsonSchema" Version="10.7.2" />
<PackageReference Include="JsonSchema.Net" Version="3.2.1" />
</ItemGroup>

</Project>
27 changes: 27 additions & 0 deletions ...ft.PowerShell.Commands.Utility/commands/utility/JsonSchemaReferenceResolutionException.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

#nullable enable

using System;

namespace Microsoft.PowerShell.Commands;

/// <summary>
/// Thrown during evaluation of <see cref="TestJsonCommand"/> when an attempt
/// to resolve a <code>$ref</code> or <code>$dynamicRef</code> fails.
/// </summary>
internal class JsonSchemaReferenceResolutionException : Exception
{
/// <summary>
/// Initializes a new instance of the <see cref="JsonSchemaReferenceResolutionException"/> class.
/// </summary>
/// <param name="innerException">
/// The exception that is the cause of the current exception, or a null reference
/// (<code>Nothing</code> in Visual Basic) if no inner exception is specified.
/// </param>
public JsonSchemaReferenceResolutionException(Exception innerException)
: base(message: null, innerException)
{
}
}
120 changes: 75 additions & 45 deletions src/Microsoft.PowerShell.Commands.Utility/commands/utility/TestJsonCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
using System.Globalization;
using System.IO;
using System.Management.Automation;
using System.Reflection;
using System.Runtime.ExceptionServices;
using System.Net.Http;
using System.Security;
using Newtonsoft.Json.Linq;
using NJsonSchema;
using System.Text.Json;
using System.Text.Json.Nodes;
using Json.Schema;

namespace Microsoft.PowerShell.Commands
{
Expand Down Expand Up @@ -52,31 +52,38 @@ public class TestJsonCommand : PSCmdlet
private JsonSchema _jschema;

/// <summary>
/// Process all exceptions in the AggregateException.
/// Unwrap TargetInvocationException if any and
/// rethrow inner exception without losing the stack trace.
/// Prepare a JSON schema.
/// </summary>
/// <param name="e">AggregateException to be unwrapped.</param>
/// <returns>Return value is unreachable since we always rethrow.</returns>
private static bool UnwrapException(Exception e)
protected override void BeginProcessing()
{
if (e.InnerException != null && e is TargetInvocationException)
{
ExceptionDispatchInfo.Capture(e.InnerException).Throw();
}
else
SchemaRegistry.Global.Fetch = static uri =>
{
ExceptionDispatchInfo.Capture(e).Throw();
}
try
{
string text;
switch (uri.Scheme)
{
case "http":
case "https":
text = new HttpClient().GetStringAsync(uri).Result;
break;
case "file":
var filename = Uri.UnescapeDataString(uri.AbsolutePath);
text = File.ReadAllText(filename);
break;
default:
throw new FormatException(
$"URI scheme '{uri.Scheme}' is not supported. Only HTTP(S) and local file system URIs are allowed.");
}

return true;
}
return JsonSerializer.Deserialize<JsonSchema>(text);
}
catch (Exception e)
{
throw new JsonSchemaReferenceResolutionException(e);
}
};

/// <summary>
/// Prepare a JSON schema.
/// </summary>
protected override void BeginProcessing()
{
string resolvedpath = string.Empty;

try
Expand All @@ -85,25 +92,25 @@ protected override void BeginProcessing()
{
try
{
_jschema = JsonSchema.FromJsonAsync(Schema).Result;
_jschema = JsonSchema.FromText(Schema);
}
catch (AggregateException ae)
catch (JsonException e)
{
// Even if only one exception is thrown, it is still wrapped in an AggregateException exception
// https://docs.microsoft.com/en-us/dotnet/standard/parallel-programming/exception-handling-task-parallel-library
ae.Handle(UnwrapException);
Exception exception = new(TestJsonCmdletStrings.InvalidJsonSchema, e);
WriteError(new ErrorRecord(exception, "InvalidJsonSchema", ErrorCategory.InvalidData, Schema));
}
}
else if (SchemaFile != null)
{
try
{
resolvedpath = Context.SessionState.Path.GetUnresolvedProviderPathFromPSPath(SchemaFile);
_jschema = JsonSchema.FromFileAsync(resolvedpath).Result;
_jschema = JsonSchema.FromFile(resolvedpath);
}
catch (AggregateException ae)
catch (JsonException e)
{
ae.Handle(UnwrapException);
Exception exception = new(TestJsonCmdletStrings.InvalidJsonSchema, e);
WriteError(new ErrorRecord(exception, "InvalidJsonSchema", ErrorCategory.InvalidData, SchemaFile));
}
}
}
Expand Down Expand Up @@ -140,26 +147,49 @@ protected override void ProcessRecord()

try
{
var parsedJson = JToken.Parse(Json);
var parsedJson = JsonNode.Parse(Json);

if (_jschema != null)
{
var errorMessages = _jschema.Validate(parsedJson);
if (errorMessages != null && errorMessages.Count != 0)
{
result = false;

Exception exception = new(TestJsonCmdletStrings.InvalidJsonAgainstSchema);

foreach (var message in errorMessages)
var validationResults = _jschema.Validate(parsedJson, new ValidationOptions { OutputFormat = OutputFormat.Basic });
result = validationResults.IsValid;
if (!result)
{
ErrorRecord errorRecord = new(exception, "InvalidJsonAgainstSchema", ErrorCategory.InvalidData, null);
errorRecord.ErrorDetails = new ErrorDetails(message.ToString());
WriteError(errorRecord);
Exception exception = new(TestJsonCmdletStrings.InvalidJsonAgainstSchema);

if (validationResults.Message != null)
{
ErrorRecord errorRecord = new(exception, "InvalidJsonAgainstSchema", ErrorCategory.InvalidData, null);
var message = $"{validationResults.Message} at {validationResults.InstanceLocation}";
errorRecord.ErrorDetails = new ErrorDetails(message);
WriteError(errorRecord);
}

if (validationResults.HasNestedResults)
{
foreach (var nestedResult in validationResults.NestedResults)
{
if (nestedResult.Message == null)
{
continue;
}

ErrorRecord errorRecord = new(exception, "InvalidJsonAgainstSchema", ErrorCategory.InvalidData, null);
var message = $"{nestedResult.Message} at {nestedResult.InstanceLocation}";
errorRecord.ErrorDetails = new ErrorDetails(message);
WriteError(errorRecord);
}
}
}
}
}
}
catch (JsonSchemaReferenceResolutionException jsonExc)
{
result = false;

Exception exception = new(TestJsonCmdletStrings.InvalidJsonSchema, jsonExc);
WriteError(new ErrorRecord(exception, "InvalidJsonSchema", ErrorCategory.InvalidData, _jschema));
}
Comment on lines +186 to +192
Copy link
Collaborator

@iSazonov iSazonov Sep 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we a test for the branch?

Copy link
Contributor Author

@gregsdennis gregsdennis Sep 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the cases where a referenced schema fails to load (and there are several) will flow through this catch.

Copy link
Collaborator

@iSazonov iSazonov Sep 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see tests where we looks JsonSchemaReferenceResolutionException

Copy link
Contributor Author

@gregsdennis gregsdennis Sep 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to. That's an internal implementation detail. It's presented to the user as "invalid JSON schema." All of these cases are covered.

Copy link
Collaborator

@iSazonov iSazonov Sep 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We throw InvalidJsonSchema in 3 places - for file, for text and if load from a reference - we haven't a test for the last. So we could create a string with valid schema but with bad reference (to non-existent file) and in InvalidJsonSchema exception check inner exception name for JsonSchemaReferenceResolutionException.

Copy link
Contributor Author

@gregsdennis gregsdennis Sep 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would just be adding an assertion to an existing test. I can do that, but I don't think that it warrants new tests.

Copy link
Contributor Author

@gregsdennis gregsdennis Sep 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an example where inner exceptions are being checked in powershell? I only see assertions on the error output.

$errorVar[0].FullyQualifiedErrorId | Should -BeExactly "InvalidJsonAgainstSchema,Microsoft.PowerShell.Commands.TestJsonCommand"

This doesn't mention the exceptions that were thrown.

Copy link
Contributor Author

@gregsdennis gregsdennis Sep 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We throw InvalidJsonSchema in 3 places - for file, for text and if load from a reference - we haven't a test for the last.

Test-Json throw if a schema from file is invalid which throws an error because invalid_schema_definitions.json is invalid JSON is failing to load a reference. Granted, it's failing to resolve the reference because the referenced file is invalid JSON, but the reference resolution still fails, triggering a JsonSchemaReferenceResolutionException, which is caught on line 186 and repackaged as an "InvalidJsonSchema" error for the user.

catch (Exception exc)
{
result = false;
Expand Down
Loading