Sockets.tests: TcpReceiveSendGetsCanceledByDispose: remove Timeout.

[tmds]

This timeout is not needed and masks a more useful exception thrown
by the test.

To help debug #52597.

@antonfirsov ptal

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This timeout is not needed and masks a more useful exception thrown
by the test.

To help debug #52597.

@antonfirsov ptal

Author:	tmds
Assignees:	-
Labels:	area-System.Net.Sockets
Milestone:	-

[antonfirsov]

Are you sure this won't turn them into infinite "Long Running" tests timing out the whole System.Net.Sockets.Tests workitem? This is what I would expect from this change, am I missing something?

If memory serves well, this happened in the past, which is the reason why I added the timeout.

[tmds]

Are you sure this won't turn them into infinite "Long Running" tests timing out the whole System.Net.Sockets.Tests workitem?

You're right. I missed that this code was refactored. This used to throw on timeout here: 5a4483e#diff-2644f228b9c0083b38290d3faabc8d69d32a0372220463c42b7f4ac14c29a77cL1035.

I'll make some more changes.

[antonfirsov]

WaitAsync should be equivalent to that check, I suspect the timeout might be happening at other places (ReceiveAsync/SendAsync tasks which are not being cancelled by timeouts?).

[tmds]

@antonfirsov I've added a WaitAsync on each await in the test.

[antonfirsov]

@tmds do you need this to be merged to see if you are able to unmask the errors with the change?

[tmds]

I think it's more meaningful for everyone that the operation times out rather than the whole test.

[antonfirsov]

LGTM, thanks!

undo approval?

[antonfirsov]

Haven't noticed first, but timeout is still missing at this line:

runtime/src/libraries/System.Net.Sockets/tests/FunctionalTests/SendReceive/SendReceive.cs

Line 1029 in ba88131

SendAsync(socket1, buffer).GetAwaiter().GetResult();

Better to go safe!

[antonfirsov]

/azp run runtime

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[stephentoub]

This could also just be:

.Wait(TimeSpan.FromMilliseconds(TestSettings.PassingTestTimeout));

If that fails it'll end up throwing an AggregateException, but I don't see anything paying attention to the exact exception type anyway...

[antonfirsov]

LGTM.

[antonfirsov]

Test failures are unrelated. I'm merging this as-is, since the change is definitely an improvement.