Skip to content
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

Jump to bottom

Create new AssertContentsAgainstJsonNet to avoid calling NormalizeToJsonNetFormat when not necessary. #32351

Closed
ahsonkhan opened this issue Feb 15, 2020 · 0 comments · Fixed by #34931
Closed

Create new AssertContentsAgainstJsonNet to avoid calling NormalizeToJsonNetFormat when not necessary. #32351

ahsonkhan opened this issue Feb 15, 2020 · 0 comments · Fixed by #34931
Labels
area-System.Text.Json help wanted [up-for-grabs] Good issue for external contributors test-enhancement Improvements of test source code
Milestone
5.0.0

Comments

@ahsonkhan
Copy link
Contributor

ahsonkhan commented Feb 15, 2020 •
edited
Loading

There are subtle differences between Newtonsoft.Json and S.T.Json on how characters are escaped (upper case hex digits vs lower case), etc. This requires helpers for testing. Capturing the TODO into an issue: dotnet/corefx#39900 (comment).

runtime/src/libraries/System.Text.Json/tests/JsonTestHelper.cs

Lines 721 to 723 in d4b06b1

// Temporary hack until we can use the same escape algorithm on both sides and make sure we want uppercase hex.
// Todo: create new AssertContentsAgainJsonNet to avoid calling NormalizeToJsonNetFormat when not necessary.
Assert.Equal(expectedValue.NormalizeToJsonNetFormat(), value.NormalizeToJsonNetFormat());

runtime/src/libraries/System.Text.Json/tests/JsonTestHelper.cs

Lines 743 to 745 in d4b06b1

// Temporary hack until we can use the same escape algorithm on both sides and make sure we want uppercase hex.
// Todo: create new AssertContentsNotEqualAgainJsonNet to avoid calling NormalizeToJsonNetFormat when not necessary.
Assert.NotEqual(expectedValue.NormalizeToJsonNetFormat(), value.NormalizeToJsonNetFormat());

cc @steveharter
@ahsonkhan ahsonkhan added area-System.Text.Json test-enhancement Improvements of test source code labels Feb 15, 2020
@ahsonkhan ahsonkhan added this to the 5.0 milestone Feb 15, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Feb 15, 2020
@ahsonkhan ahsonkhan removed the untriaged New issue has not been triaged by the area owner label Feb 15, 2020
@ahsonkhan ahsonkhan mentioned this issue Feb 15, 2020
Merged
@layomia layomia added the help wanted [up-for-grabs] Good issue for external contributors label Apr 7, 2020
marcusturewicz added a commit to marcusturewicz/runtime that referenced this issue Apr 14, 2020
@marcusturewicz
Adds AssertContentsAgainstJsonNet to avoid unnecessary calls
101bcd9 
Fixes dotnet#32351
@marcusturewicz marcusturewicz mentioned this issue Apr 14, 2020
Merged
layomia pushed a commit that referenced this issue Apr 15, 2020
@marcusturewicz
Adds AssertContentsAgainstJsonNet to avoid unnecessary calls (#34931)
2eef5a3 
* Adds AssertContentsAgainstJsonNet to avoid unnecessary calls

Fixes #32351

* Remove temporary hack comments
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area-System.Text.Json help wanted [up-for-grabs] Good issue for external contributors test-enhancement Improvements of test source code
Projects
None yet
Milestone
5.0.0
Development

Successfully merging a pull request may close this issue.

Adds AssertContentsAgainstJsonNet to avoid unnecessary calls marcusturewicz/runtime
3 participants
@ahsonkhan @layomia @Dotnet-GitSync-Bot