Corefxlab: Question about System.IO.Pipelines SocketFacts Ping Pong tests

[ahsonkhan]

Should this be "PONG"?

https://github.com/dotnet/corefxlab/blob/master/tests/System.IO.Pipelines.Tests/SocketsFacts.cs#L179
C# ArraySegment<byte> _ping = new ArraySegment<byte>(Encoding.ASCII.GetBytes("PING")); ArraySegment<byte> _pong = new ArraySegment<byte>(Encoding.ASCII.GetBytes("PING"));
_pong is not used at all in PingClient

Similarly:
https://github.com/dotnet/corefxlab/blob/master/tests/System.IO.Pipelines.Tests/SocketsFacts.cs#L237

cc @davidfowl, @pakrym

[pakrym]

It's not used in that test and can be safely deleted.

[ahsonkhan]

It is used in PongServer

Should the string still be "PING"?

[pakrym]

Nothing checks it, so it can be arbitrary.

[ahsonkhan]

If I set it to "PONG", the test fails:

Failed System.IO.Pipelines.Tests.SocketsFacts.RunStressPingPongTest_Libuv
Error Message:
Assert.Equal() Failure
Expected: 500
Actual: 1
Stack Trace:
at System.IO.Pipelines.Tests.SocketsFacts.d__4.MoveNext() in D:\GitHub\Fork\corefxlab\tests\System.IO.Pipelines.Tests\SocketsFacts.cs:line 131
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.IO.Pipelines.Tests.SocketsFacts.d__4.MoveNext() in D:\GitHub\Fork\corefxlab\tests\System.IO.Pipelines.Tests\SocketsFacts.cs:line 138
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)

[pakrym]

Yep, I was wrong, it wasn't entire test, just a helper method, sorry. We can change it to be pong.

[KodrAus]

Yeh it checks the pong is equal to ping here. Should probably just check it's equal to _pong.

[davidfowl]

@pakrym are you going to remove these tests?

[pakrym]

We'll keep them for a while in corefxlab running against corefx packages to make sure things do not regress abruptly.

[davidfowl]

Ok I'm going to close this bug