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

testing: increase stability by removing thread parallelism #292

Merged
merged 1 commit into from
Mar 5, 2016

Conversation

posborne
Copy link
Member

@posborne posborne commented Mar 5, 2016

Currently, several of the tests are failing intermittently. After
some research it appears that these failures only occur when thread
parallelism is enabled (as is the case by default). To test, I just
ran the failing tests over and over. I would consistently see errors
when running the following:

$ while true; do target/debug/test-7ec4d9681e812f6a; done

When I forced single threaded execution, I no longer saw failures:

$ while true; do RUST_TEST_THREADS=1 target/debug/test-7ec4d9681e812f6a; done

I was mostly looking at the test_unistd failures which make calls out
to fork() and then make subsequent calls to wait(). In that case there
is one parent and the wait() called could (and frequently does) get some
random child pid back because it just happened to terminate. That is
why when one of the test fails so does the other one.

I couldn't think of an obvious fix other than preventing thread
parallelism in the short term. The tests still run very quickly.

#251

Signed-off-by: Paul Osborne osbpau@gmail.com

Currently, several of the tests are failing intermittently.  After
some research it appears that these failures only occur when thread
parallelism is enabled (as is the case by default).  To test, I just
ran the failing tests over and over.  I would consistently see errors
when running the following:

    $ while true; do target/debug/test-7ec4d9681e812f6a; done

When I forced single threaded execution, I no longer saw failures:

    $ while true; do RUST_TEST_THREADS=1 target/debug/test-7ec4d9681e812f6a; done

I was mostly looking at the test_unistd failures which make calls out
to fork() and then make subsequent calls to wait().  In that case there
is one parent and the wait() called could (and frequently does) get some
random child pid back because it just happened to terminate.  That is
why when one of the test fails so does the other one.

I couldn't think of an obvious fix other than preventing thread
parallelism in the short term.  The tests still run very quickly.

nix-rust#251

Signed-off-by: Paul Osborne <osbpau@gmail.com>
@posborne
Copy link
Member Author

posborne commented Mar 5, 2016

r? @kamalmarhubi

@kamalmarhubi
Copy link
Member

This is at least a good stopgap for spurious CI failures.

@homu r+

@homu
Copy link
Contributor

homu commented Mar 5, 2016

📌 Commit 896a446 has been approved by kamalmarhubi

@kamalmarhubi
Copy link
Member

I was mostly looking at the test_unistd failures which make calls out
to fork() and then make subsequent calls to wait(). In that case there
is one parent and the wait() called could (and frequently does) get some
random child pid back because it just happened to terminate. That is
why when one of the test fails so does the other one.

This is great to know. I hadn't looked at why there were failures.

@posborne
Copy link
Member Author

posborne commented Mar 5, 2016

Agreed that this still doesn't feel ideal. Unit tests are a pretty difficult thing to achieve for libs like this (and calling the actual fork() is obviously not a "good" test in the traditional sense). For testing C code, I have made use of linker flags to wrap actual system calls with mocks but I haven't been able to think of a good, portable way of handling this in rust.

A generic mocking utilty for testing could be pretty useful across the ecosystem if we can think of a good way of handling.

http://stackoverflow.com/questions/3662856/how-to-reimplement-or-wrap-a-syscall-function-in-linux

@homu
Copy link
Contributor

homu commented Mar 5, 2016

⚡ Test exempted - status

@homu homu merged commit 896a446 into nix-rust:master Mar 5, 2016
homu added a commit that referenced this pull request Mar 5, 2016
…hubi

testing: increase stability by removing thread parallelism

Currently, several of the tests are failing intermittently.  After
some research it appears that these failures only occur when thread
parallelism is enabled (as is the case by default).  To test, I just
ran the failing tests over and over.  I would consistently see errors
when running the following:

    $ while true; do target/debug/test-7ec4d9681e812f6a; done

When I forced single threaded execution, I no longer saw failures:

    $ while true; do RUST_TEST_THREADS=1 target/debug/test-7ec4d9681e812f6a; done

I was mostly looking at the test_unistd failures which make calls out
to fork() and then make subsequent calls to wait().  In that case there
is one parent and the wait() called could (and frequently does) get some
random child pid back because it just happened to terminate.  That is
why when one of the test fails so does the other one.

I couldn't think of an obvious fix other than preventing thread
parallelism in the short term.  The tests still run very quickly.

#251

Signed-off-by: Paul Osborne <osbpau@gmail.com>
@posborne posborne deleted the test-stability-improvements branch March 5, 2016 20:28
kamalmarhubi added a commit to kamalmarhubi/nix-rust that referenced this pull request Mar 8, 2016
After nix-rust#292 was merged, this flakiness remained. I observe it only on
Darwin, hence the targetted disabling until there's been more
investigation.
homu added a commit that referenced this pull request Mar 8, 2016
tests: Disable test_sigwait on apple platforms

After #292 was merged, this flakiness remained. I observe it only on
Darwin, hence the targetted disabling until there's been more
investigation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants