-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
Disable EnvPosixTest.RunImmediately, add EnvPosixTest.RunEventually #4126
Conversation
The original `EnvPosixTest.RunImmediately` assumes that after scheduling a background thread, the thread is guaranteed to complete after 0.1 second. I do not know about any non-real-time OS/runtime providing this guarantee. Nor does C++11 standard say anything about this. In fact, we have observed this test failure multiple times on appveyor, and we haven't been able to reproduce the failure deterministically. Therefore, I disable this test for now until we know for sure how it used to fail. Instead, I add another test `EnvPosixTest.RunEventually` that checks that a thread will be scheduled eventually. Test plan: ``` $make clean && make -j16 $./env_test --gtest_filter=EnvPosixTest.RunImmediately # runs 0 test $./env_test --gtest_filter=EnvPosixTest.RunEventually # runs 1 test successfully ```
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.
@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
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!
EnvPosixTest.RunImmediately
, add EnvPosixTest.RunEventually
.
EnvPosixTest.RunImmediately
, add EnvPosixTest.RunEventually
.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.
@riversand963 is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
…acebook#4126) Summary: The original `EnvPosixTest.RunImmediately` assumes that after scheduling a background thread, the thread is guaranteed to complete after 0.1 second. I do not know about any non-real-time OS/runtime providing this guarantee. Nor does C++11 standard say anything about this in the documentation of `std::thread`. In fact, we have observed this test failure multiple times on appveyor, and we haven't been able to reproduce the failure deterministically. Therefore, I disable this test for now until we know for sure how it used to fail. Instead, I add another test `EnvPosixTest.RunEventually` that checks that a thread will be scheduled eventually. Pull Request resolved: facebook#4126 Differential Revision: D8827086 Pulled By: riversand963 fbshipit-source-id: abc5cb655f90d50b791493da5eeb3716885dfe93
The original
EnvPosixTest.RunImmediately
assumes that after schedulinga background thread, the thread is guaranteed to complete after 0.1 second.
I do not know about any non-real-time OS/runtime providing this guarantee. Nor
does C++11 standard say anything about this in the documentation of
std::thread
.In fact, we have observed this test failure multiple times on appveyor, and we
haven't been able to reproduce the failure deterministically. Therefore,
I disable this test for now until we know for sure how it used to fail.
Instead, I add another test
EnvPosixTest.RunEventually
that checks thata thread will be scheduled eventually.
Test plan: