-
Notifications
You must be signed in to change notification settings - Fork 351
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
Set '--test-threads' option to 1 in unit tests #2685
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2685 +/- ##
=======================================
Coverage 65.40% 65.41%
=======================================
Files 133 133
Lines 16942 16942
=======================================
+ Hits 11081 11082 +1
+ Misses 5861 5860 -1 |
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.
LGTM.
IIUC, the process will still have 2 threads, one driving the testing framework and another running the actual test. But the thread running the framework will be sleepping the whole time the test thread is running, so it should be fine.
Yes, I think that is correct. But if we are to use cargo test, or anything else, I don't think that can be avoided, so limiting the thread to 1 was the best option I could think of :) Another approach was as utam0k suggested, to separate the hanging tests with some kind of feature, and run them single threaded separately, but I found that some of them already had a |
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.
The code is a bit changed. However, this change has been based on a lot of your research. And it solves a critical problem that bothers Youki's maintainer. I appreciate you 😍
Another try at #2615 and #2144
done with ref #2615 (comment)
Instead of splitting the tests by configs, tried setting the number of test threads to 1, so the tests are always run one at a time. There is no considerable change in time, the basic check CI (whole) takes about 5-6 minutes now instead of 3 minutes previously. I ran it 10 times, and it didn't fail in any attempt : https://github.com/YJDoc2/youki/actions/workflows/basic.yml?query=branch%3Atests%2Ffix-for-2144-v2
This is a much simpler solution for the issue, and as mentioned in original problem, the root cause comes from tests running in multiple threads, and should not occur in actual use ; so doing this seems a better option than code changes, if this works.