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.

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

Enhance error message: Test is not runnable in single-threaded context. Timeout #2087

Closed
PDHCoder opened this issue Mar 21, 2017 · 4 comments · Fixed by #2093
Closed

Enhance error message: Test is not runnable in single-threaded context. Timeout #2087

PDHCoder opened this issue Mar 21, 2017 · 4 comments · Fixed by #2093

Comments

@PDHCoder
Copy link

PDHCoder commented Mar 21, 2017

When using the SingleThreadedAttribute and a TimeOutAttribute on a class, you get the error message:
"Test is not runnable in single-threaded context. Timeout".
It took quite a while to figure out what went wrong (I initially thought that my TestFixtureSetup code took too long).

Suggested error message:
"The SingleThreaded attribute and the Timeout attribute cannot be combined".

@CharliePoole
Copy link
Member

Good point! The message was originally an internal Debug message, but ended up being shown to the user as an error message. In its context, Timeout is the value of an internal enumeration that gives the reason for needing to create a separate thread. It could also say RequiresThread or DifferentApartment.

For ease of understanding, I think we need to either craft a separate message for each case or use a single message that simply says "You specified an attribute that requires a test to run on a separate thread in a Single-Threaded fixture."

The suggested message "The SingleThreaded attribute and the Timeout attribute cannot be combined" might be confusing if the first attribute is on the fixture and the second is on the method.

@PDHCoder
Copy link
Author

Then I suggest that you craft a separate message for each case, because I wouldn't have gotten any wiser with the message "You specified an attribute that requires a test to run on a separate thread in a Single-Threaded fixture." (I would not make the link from 'an attribute' the the Timeout attribute).

By the way, what I actually wanted to achieve with the SingleThreaded attribute is that the OneTimeSetup code and the actual test ran on the same thread. I would not object that the NUnit framework created a separate thread for monitoring the Timeout. I think that Timeout monitoring is really valuable on a build server that executes the unit tests (to prevent the build process from freezing). So I would even more like a solution in which the SingleThreaded attribute and the TimeOut attribute can be combined.

@CharliePoole
Copy link
Member

Unfortunately, that's not what the TimeoutAttribute does. It causes the test itself to be run on a separate thread, so that thread can be cancelled without fixing anything else.

The alternate approach that you suggest is possible. We could have created a "watcher" thread that would keep track of the test and cancel it. However, we would then be cancelling the worker thread that is pulling tests from a queue and running them. You might start with 4 workers but as tests timed out, you would have 3, then 2, then 1, finally none!

The long term approach to this is to have workers created and destroyed dynamically. Then when a test timed out, we could immediately start a new worker thread to replace the one cancelled. Most likely, we would implement this via a thread pool. Currently, however, the specified number of workers are created statically and this will be a rather big change. This is covered in issue #1363.

For purposes of this issue, we'll fix the message as you suggest.

@CharliePoole
Copy link
Member

This issue was fixed and closed by PR #2093. The fix only deals with clarifying the message, not when the message appears. It will continue to appear whenever a test assembly contains a single-threaded fixture and has a non-zero default timeout specified. I offered some alternative add-on changes as possibilities in a comment on the PR. If anyone wants to follow up on those ideas, they should file a new issue.

@rprouse rprouse modified the milestone: 3.7 May 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants