Skip to content

Address JsonElement doc and test feedback #43832

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

Merged
steveharter merged 2 commits into from
Oct 30, 2020
Merged

Address JsonElement doc and test feedback #43832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de0c074
Address JsonElement doc and test feedback
steveharter Oct 26, 2020
93da7fa
Add missing Asserts, revert some doc
steveharter Oct 29, 2020
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
2 changes: 1 addition & 1 deletion src/libraries/System.Text.Json/src/System/Text/Json/Document/JsonElement.Parse.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public static JsonElement ParseValue(ref Utf8JsonReader reader)
/// <param name="reader">The reader to read.</param>
/// <param name="element">Receives the parsed element.</param>
/// <returns>
/// <see langword="true"/> if a value was read and parsed into a JsonElement,
/// <see langword="true"/> if a value was read and parsed into a JsonElement;
/// <see langword="false"/> if the reader ran out of data while parsing.
/// All other situations result in an exception being thrown.
/// </returns>
Expand Down
73 changes: 63 additions & 10 deletions src/libraries/System.Text.Json/tests/JsonElementParseTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ public static IEnumerable<object[]> ElementParseCases
yield return new object[] { "true", JsonValueKind.True };
yield return new object[] { "false", JsonValueKind.False };
yield return new object[] { "\"MyString\"", JsonValueKind.String };
yield return new object[] { @"""\u0033\u002e\u0031""", JsonValueKind.String }; // "3.12"
yield return new object[] { "1", JsonValueKind.Number };
yield return new object[] { "3.125e7", JsonValueKind.Number };
yield return new object[] { "{}", JsonValueKind.Object };
yield return new object[] { "[]", JsonValueKind.Array };
}
Expand All @@ -29,6 +32,7 @@ public static void ParseValue(string json, JsonValueKind kind)

JsonElement? element = JsonElement.ParseValue(ref reader);
Assert.Equal(kind, element!.Value.ValueKind);
Assert.Equal(json.Length, reader.BytesConsumed);
}

[Theory]
Expand All @@ -40,6 +44,7 @@ public static void TryParseValue(string json, JsonValueKind kind)
bool success = JsonElement.TryParseValue(ref reader, out JsonElement? element);
Assert.True(success);
Assert.Equal(kind, element!.Value.ValueKind);
Assert.Equal(json.Length, reader.BytesConsumed);
}

public static IEnumerable<object[]> ElementParsePartialDataCases
Expand All @@ -49,6 +54,7 @@ public static IEnumerable<object[]> ElementParsePartialDataCases
yield return new object[] { "\"MyString"};
yield return new object[] { "{" };
yield return new object[] { "[" };
yield return new object[] { " \n" };
}
}

Expand All @@ -58,12 +64,21 @@ public static void ParseValuePartialDataFail(string json)
{
var reader = new Utf8JsonReader(Encoding.UTF8.GetBytes(json));

Exception ex;
try
{
JsonElement.ParseValue(ref reader);
Assert.True(false, "Expected exception.");
ex = null;
}
catch (JsonException) { }
catch (Exception e)
Copy link
Contributor

@ahsonkhan ahsonkhan Oct 26, 2020

Choose a reason for hiding this comment

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

Should this be:

Suggested change
catch (Exception e)
catch (JsonException e)

Copy link
Contributor Author

@steveharter steveharter Oct 29, 2020

Choose a reason for hiding this comment

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

It could be, but I followed the pattern in JsonDocumentTests, which I believe wants to fail with a nice Assert instead of a unhandled exception.

{
ex = e;
}

Assert.NotNull(ex);
Assert.IsAssignableFrom<JsonException>(ex);

Assert.Equal(0, reader.BytesConsumed);
Copy link
Contributor

@ahsonkhan ahsonkhan Oct 26, 2020

Choose a reason for hiding this comment

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

Missing asserts on ex.

Copy link
Contributor Author

@steveharter steveharter Oct 29, 2020

Choose a reason for hiding this comment

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

Yep. Thanks

}

[Theory]
Expand All @@ -72,12 +87,21 @@ public static void TryParseValuePartialDataFail(string json)
{
var reader = new Utf8JsonReader(Encoding.UTF8.GetBytes(json));

Exception ex;
try
{
JsonElement.TryParseValue(ref reader, out JsonElement? element);
Assert.True(false, "Expected exception.");
ex = null;
}
catch (Exception e)
{
ex = e;
}
catch (JsonException) { }

Assert.NotNull(ex);
Assert.IsAssignableFrom<JsonException>(ex);

Assert.Equal(0, reader.BytesConsumed);
}

[Theory]
Expand All @@ -86,12 +110,21 @@ public static void ParseValueOutOfData(string json)
{
var reader = new Utf8JsonReader(Encoding.UTF8.GetBytes(json), isFinalBlock: false, new JsonReaderState());

Exception ex;
try
{
JsonElement.ParseValue(ref reader);
Assert.True(false, "Expected exception.");
ex = null;
}
catch (Exception e)
{
ex = e;
}
catch (JsonException) { }

Assert.NotNull(ex);
Assert.IsAssignableFrom<JsonException>(ex);

Assert.Equal(0, reader.BytesConsumed);
}

[Theory]
Expand All @@ -100,6 +133,8 @@ public static void TryParseValueOutOfData(string json)
{
var reader = new Utf8JsonReader(Encoding.UTF8.GetBytes(json), isFinalBlock: false, new JsonReaderState());
Assert.False(JsonElement.TryParseValue(ref reader, out JsonElement? element));
Assert.Null(element);
Assert.Equal(0, reader.BytesConsumed);
}

public static IEnumerable<object[]> ElementParseInvalidDataCases
Expand All @@ -117,12 +152,21 @@ public static void ParseValueInvalidDataFail(string json)
{
var reader = new Utf8JsonReader(Encoding.UTF8.GetBytes(json));

Exception ex;
try
{
JsonElement.ParseValue(ref reader);
Assert.True(false, "Expected exception.");
ex = null;
}
catch (JsonException) { }
catch (Exception e)
{
ex = e;
}

Assert.NotNull(ex);
Assert.IsAssignableFrom<JsonException>(ex);

Assert.Equal(0, reader.BytesConsumed);
}

[Theory]
Expand All @@ -131,12 +175,21 @@ public static void TryParseValueInvalidDataFail(string json)
{
var reader = new Utf8JsonReader(Encoding.UTF8.GetBytes(json));

Exception ex;
try
{
JsonElement.TryParseValue(ref reader, out JsonElement? element);
Assert.True(false, "Expected exception.");
ex = null;
}
catch (Exception e)
{
ex = e;
}
catch (JsonException) { }

Assert.NotNull(ex);
Assert.IsAssignableFrom<JsonException>(ex);

Assert.Equal(0, reader.BytesConsumed);
}
}
}