Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Improve SocketOutputTests #1047

Merged
merged 1 commit into from
Aug 15, 2016
Merged

Improve SocketOutputTests #1047

merged 1 commit into from
Aug 15, 2016

Conversation

halter73
Copy link
Member

- This should increase reliability/determinism by removing timeouts.
@mikeharder
Copy link
Contributor

I agree with changing the test methods to async Task and removing the 1-second timeouts. However, why is the SynchronousThreadPool required? Wouldn't the tests work just as well with an async threadpool?

@halter73
Copy link
Member Author

@mikeharder We are verifying the tasks are in the correct state immediately after WriteAsync is called or the uv_write callback is called depending on the test.

The default IThreadPool, LoggingThreadPool is used to complete these tasks which basically runs ThreadPool.QueueUserWorkItem(tcs => tcs.TrySetResult(null), tcs); with some additional casting and logging. Same goes for errors and cancellation.

This means that we can't verify the task gets completed or not without a timeout if we use the default threadpool. At that point we have to pick a consistent timeout which was 1 second before, but even that was not long enough on some CI runs. Most of the time, verifying task completion happens fairly quickly (but sometimes over a second apparently), but verifying task noncompletion will take the full lenght of the time.

@mikeharder
Copy link
Contributor

If you need to verify non-completion, I agree it's better to use a SynchronousThreadPool than to wait an arbitrary period of time.

@mikeharder
Copy link
Contributor

:shipit:

@halter73 halter73 merged commit 8f4cc30 into dev Aug 15, 2016
@halter73 halter73 deleted the halter73/1002 branch August 15, 2016 22:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants