Use relaxed JavascriptEncoder in S.T.JSON specific tests to improve coverage. by ahsonkhan · Pull Request #39560 · dotnet/corefx

ahsonkhan · 2019-07-17T01:00:58Z

coverage.

src/System.Text.Json/tests/Serialization/Value.WriteTests.cs

src/System.Text.Json/tests/Utf8JsonWriterTests.cs

src/System.Text.Json/tests/Serialization/Value.WriteTests.cs

steveharter · 2019-07-17T18:39:47Z

src/System.Text.Json/tests/Utf8JsonWriterTests.cs

+            var ms = new MemoryStream();
+            TextWriter streamWriter = new StreamWriter(ms, new UTF8Encoding(false), 1024, true);
+
+            var json = new JsonTextWriter(streamWriter)


We should discuss whether this is the best way to get expected strings going forward. Seems to work by coincidence in this case (same escaping rules). In other cases we have started to diverge (e.g. casing).

are disposed.

ahsonkhan · 2019-07-18T01:44:37Z

Any other feedback?

bartonjs · 2019-07-18T15:28:13Z

src/System.Text.Json/tests/JsonElementWriteTests.cs

+                        Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
+                    };
+
+                    using var writer = new Utf8JsonWriter(buffer, options);


Personally, I vastly prefer "block statement using". This using ends after AssertContents, but following the least-need principle the using should end after (or instead-of) writer.Flush. (The new using statement is particularly bad in async methods, where it can make the using straddle an await it didn't need to)

So, replacing all of the usings with scoped blocks would make me happier, but it's not critical.

ericstj · 2019-07-25T15:29:13Z

@ahsonkhan are you planning to get this one in?

…axedEscaper

ghost · 2019-07-29T03:29:47Z

Hello @ahsonkhan!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

ahsonkhan · 2019-07-29T04:16:01Z

Test failure is unrelated:
https://mc.dot.net/#/user/dotnet-bot/pr~2Fdotnet~2Fcorefx~2Frefs~2Fpull~2F39560~2Fmerge/test~2Ffunctional~2Fcli~2F~2F/20190728.25/workItem/System.Net.Http.Functional.Tests/analysis/xunit/System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_Http2~2FHttp2_PendingReceive_SendsReset(doRead:%20True)
System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_Http2/Http2_PendingReceive_SendsReset(doRead: True)

Message :
System.IO.IOException : Unable to read data from the transport connection: Broken pipe.
---- System.Net.Sockets.SocketException : Broken pipe
Stack Trace :
   at System.Net.Security.SslStream.<WriteSingleChunk>g__CompleteAsync|210_1[TWriteAdapter](ValueTask writeTask, Byte[] bufferToReturn) in /_/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 1271
   at System.Net.Security.SslStream.WriteAsyncInternal[TWriteAdapter](TWriteAdapter writeAdapter, ReadOnlyMemory`1 buffer) in /_/src/System.Net.Security/src/System/Net/Security/SslStream.Implementation.cs:line 1521
   at System.Net.Test.Common.Http2LoopbackConnection.WriteFrameAsync(Frame frame) in /_/src/Common/tests/System/Net/Http/Http2LoopbackConnection.cs:line 87
   at System.Net.Http.Functional.Tests.HttpClientHandlerTest_Http2.<>c__DisplayClass62_0.<<Http2_PendingReceive_SendsReset>b__1>d.MoveNext() in /_/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs:line 1784
--- End of stack trace from previous location where exception was thrown ---
   at System.Threading.Tasks.TaskTimeoutExtensions.WhenAllOrAnyFailed(Task[] tasks) in /_/src/Common/tests/System/Threading/Tasks/TaskTimeoutExtensions.cs:line 83
   at System.Threading.Tasks.TaskTimeoutExtensions.WhenAllOrAnyFailed(Task[] tasks) in /_/src/Common/tests/System/Threading/Tasks/TaskTimeoutExtensions.cs:line 111
   at System.Threading.Tasks.TaskTimeoutExtensions.WhenAllOrAnyFailed(Task[] tasks, Int32 millisecondsTimeout) in /_/src/Common/tests/System/Threading/Tasks/TaskTimeoutExtensions.cs:line 71
   at System.Net.Test.Common.Http2LoopbackServer.CreateClientAndServerAsync(Func`2 clientFunc, Func`2 serverFunc, Int32 timeout) in /_/src/Common/tests/System/Net/Http/Http2LoopbackServer.cs:line 186
   at System.Net.Http.Functional.Tests.HttpClientHandlerTest_Http2.Http2_PendingReceive_SendsReset(Boolean doRead) in /_/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs:line 1736
--- End of stack trace from previous location where exception was thrown ---
----- Inner Stack Trace -----

…overage. (dotnet#39560) * Use relaxed JavascriptEncoder in S.T.JSON specific tests to improve coverage. * Add control characters as well as part of the test data. * Add JsonElement.WriteTo test and make sure all Utf8JsonWriter in tests are disposed. * Fix spacing/formatting. * Fix call to AssertEquals since it moved to JsonTestHelper.

…overage. (#39560) (#39850) * Use relaxed JavascriptEncoder in S.T.JSON specific tests to improve coverage. * Add control characters as well as part of the test data. * Add JsonElement.WriteTo test and make sure all Utf8JsonWriter in tests are disposed. * Fix spacing/formatting. * Fix call to AssertEquals since it moved to JsonTestHelper.

karelz · 2019-08-03T18:19:47Z

@ahsonkhan this specific PR went into master after last integration to release/3.0 branch, so I fixed milestone to 5.0.
It seems it was ported to 3.0 in #39850.

…overage. (dotnet/corefx#39560) * Use relaxed JavascriptEncoder in S.T.JSON specific tests to improve coverage. * Add control characters as well as part of the test data. * Add JsonElement.WriteTo test and make sure all Utf8JsonWriter in tests are disposed. * Fix spacing/formatting. * Fix call to AssertEquals since it moved to JsonTestHelper. Commit migrated from dotnet/corefx@74d26be

Use relaxed JavascriptEncoder in S.T.JSON specific tests to improve

db60310

coverage.

ahsonkhan added test enhancement Improvements of test source code area-System.Text.Json labels Jul 17, 2019

ahsonkhan added this to the 3.0 milestone Jul 17, 2019

ahsonkhan self-assigned this Jul 17, 2019

Add control characters as well as part of the test data.

e29df0f

bartonjs reviewed Jul 17, 2019

View reviewed changes

src/System.Text.Json/tests/Serialization/Value.WriteTests.cs Outdated Show resolved Hide resolved

bartonjs reviewed Jul 17, 2019

View reviewed changes

src/System.Text.Json/tests/Utf8JsonWriterTests.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jul 17, 2019

View reviewed changes

src/System.Text.Json/tests/Serialization/Value.WriteTests.cs Show resolved Hide resolved

steveharter reviewed Jul 17, 2019

View reviewed changes

ahsonkhan added 2 commits July 17, 2019 16:41

Add JsonElement.WriteTo test and make sure all Utf8JsonWriter in tests

36a0f79

are disposed.

Fix spacing/formatting.

cb66d19

bartonjs reviewed Jul 18, 2019

View reviewed changes

ahsonkhan added 2 commits July 28, 2019 20:24

Merge branch 'master' of https://github.com/dotnet/corefx into UseRel…

a8e6764

…axedEscaper

Fix call to AssertEquals since it moved to JsonTestHelper.

cac8f20

ahsonkhan added the auto-merge Automatically merge PR once CI passes. label Jul 29, 2019

ahsonkhan merged commit 74d26be into dotnet:master Jul 29, 2019

ahsonkhan deleted the UseRelaxedEscaper branch July 29, 2019 04:17

ahsonkhan mentioned this pull request Jul 29, 2019

[release/3.0] Use relaxed JavascriptEncoder in S.T.JSON specific tests to improve coverage. #39850

Merged

stephentoub mentioned this pull request Jul 29, 2019

Fix WriteLargeKeyOrValue System.Text.Json test #39857

Merged

karelz removed the test enhancement Improvements of test source code label Aug 3, 2019

karelz modified the milestones: 3.0, 5.0 Aug 3, 2019

stephentoub mentioned this pull request Aug 3, 2019

[release/3.0] Fix WriteLargeKeyOrValue System.Text.Json test #39867

Merged

benaadams mentioned this pull request Feb 1, 2020

CI not running for PRs dotnet/runtime#30297

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use relaxed JavascriptEncoder in S.T.JSON specific tests to improve coverage.#39560

Use relaxed JavascriptEncoder in S.T.JSON specific tests to improve coverage.#39560
ahsonkhan merged 6 commits intodotnet:masterfrom
ahsonkhan:UseRelaxedEscaper

ahsonkhan commented Jul 17, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

steveharter Jul 17, 2019

Uh oh!

ahsonkhan commented Jul 18, 2019

Uh oh!

bartonjs Jul 18, 2019

Uh oh!

ericstj commented Jul 25, 2019

Uh oh!

ghost commented Jul 29, 2019

Uh oh!

ahsonkhan commented Jul 29, 2019

Uh oh!

karelz commented Aug 3, 2019 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

ahsonkhan commented Jul 17, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

steveharter Jul 17, 2019

Choose a reason for hiding this comment

Uh oh!

ahsonkhan commented Jul 18, 2019

Uh oh!

bartonjs Jul 18, 2019

Choose a reason for hiding this comment

Uh oh!

ericstj commented Jul 25, 2019

Uh oh!

ghost commented Jul 29, 2019

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

Uh oh!

ahsonkhan commented Jul 29, 2019

Uh oh!

karelz commented Aug 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

karelz commented Aug 3, 2019 •

edited

Loading