-
Notifications
You must be signed in to change notification settings - Fork 661
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
Make the tests that failed to acquire locks panic explicitly #2363
Comments
Well, I just realized that we are using |
Ok, I will change it back tomorrow |
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
We have some process-wide Mutex locks in
test.rs
to ensure tests that need to modify global states will be executed sequentially:nix/test/test.rs
Lines 59 to 74 in a7db481
This should work well if the corresponding test succeeds, but once a test holding the lock failed, the thread running that test panicked, the lock got poisoned, then other tests (threads) cannot acquire the lock, and since we don't check if the lock acquisition is successful in our tests, under such scenario, those global locks won't work at all.
nix/test/test_unistd.rs
Line 43 in a7db481
Given that those global locks do not work under the above case, we can get some unexpected results from our test, e.g., see these 2 failures here, they want to wait for the child process created by themselves, but got the children of others:
In my mind, I am thinking about calling
.unwrap()
onlock()
so that the tests that cannot acquire the lock will panic explicitly, which would make things clearer, though I cannot say it is generally a better approach. cc @asomersThe text was updated successfully, but these errors were encountered: