Skip to content

Fix race in test_inherit_env #42795

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Fix race in test_inherit_env #42795

wants to merge 1 commit into from

Conversation

stepancheg
Copy link
Contributor

env tests set (and unset) randomly generated variables with names
starting with TEST. So in test_inherit_env between variable
capture and process spawn TEST* variables can be unset if env
tests executed concurrently. Thus TEST* variables must not be
checked in that test.

@stepancheg
Fix race in test_inherit_env
0bde646 
`env` tests set (and unset) randomly generated variables with names
starting with `TEST`. So in `test_inherit_env` between variable
capture and process spawn `TEST*` variables can be unset if `env`
tests executed concurrently.  Thus `TEST*` variables must not be
checked in that test.
@rust-highfive
Copy link
Contributor

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

Could this test actually be moved to a run-pass test? Most of the env tests probably should be in the long run because they're modifying global state, but it's fine to move just this one for now.

@alexcrichton alexcrichton added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 22, 2017
@stepancheg
Copy link
Contributor Author

stepancheg commented Jun 22, 2017 •
edited
Loading

Shouldn't tests which modify global state be moved to run-pass, and this test (which doesn't modify global state) kept where it is instead?

@alexcrichton
Copy link
Member

Yes, depends on how much work you're willing to do. One's much easier than the other most likely

stepancheg added a commit to stepancheg/rust that referenced this pull request Jun 24, 2017
@stepancheg
Move global vars changing tests into run-pass
45af6ee 
Should fix race rust-lang#42795
@stepancheg stepancheg mentioned this pull request Jun 24, 2017
Merged
@stepancheg stepancheg closed this Jun 24, 2017
arielb1 pushed a commit to arielb1/rust that referenced this pull request Jun 29, 2017
Rollup merge of rust-lang#42884 - stepancheg:set-env-run-pass, r=alex…
4bae0d8 
…crichton

Move global vars changing tests into run-pass

Should fix race rust-lang#42795
@orivej
Copy link

orivej commented Jun 8, 2018

Could it be that test_inherit_env is still failing because it was not moved into run-pass? (https://github.com/rust-lang/rust/blob/1.26.2/src/libstd/process.rs#L1748) Seen with Rust 1.26.2 here: https://hydra.nixos.org/build/75678845/nixlog/1, and with Rust 1.25.0 here: https://hydra.nixos.org/build/74824788/nixlog/1

@orivej orivej mentioned this pull request Jun 8, 2018
Merged
@kennytm kennytm mentioned this pull request Oct 30, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@aturon aturon

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@stepancheg @rust-highfive @alexcrichton @orivej @aturon