-
Notifications
You must be signed in to change notification settings - Fork 790
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
Asorted tests improvements #17840
Asorted tests improvements #17840
Conversation
✅ No release notes required |
This comes down to the timeout here being too tight for the CI occasionally: fsharp/tests/FSharp.Test.Utilities/CompilerAssert.fs Lines 665 to 666 in 8eec34b
Maybe it would make sense to get rid of such hardcoded timeouts altogether. In the IDE it's obvious when a test hangs and in the CI we can just use |
I used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for applying proper testing practices, @majocha. This is a really solid effort. I have also learned some multithreading stuff from this :)
They're still not perfect. Using just sync primitives often obfuscates the code. Now I think some sort of simulated time, Rx style, would make the code more clear. But at the cost of making the tests depend on yet another thing. |
Well - we're leaving the playground in the better shape we've found it 👀 |
Great improvement indeed |
Cherrypicked from #17662.
Async.Start
etc. and not observed / awaited. They will escape the confinement of currently running test case and if they throw, they bring down the whole test hostexit 0
in the codeMailbox.TryScan
because we had none outside of the core\controlMailbox script.