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

Handle RUST_TEST_NOCAPTURE in compiletest and set add if to run env #22371

Merged

Conversation

fhahn
Copy link
Contributor

@fhahn fhahn commented Feb 15, 2015

This is a patch for #17829.

In compiletest there are multiple layers which capture the output. The first layer is run_tests_console which is used to execute all tests.

Then there are some tests that contain unit tests, which by default also captures output. Therefore compiletest adds RUST_TEST_NOCAPTURE (and RUST_TEST_TASKS for completeness) to the run environment of the task.

Finally, the task used to execute a test redirects stdout and stdin. At the moment, the VERBOSE=1 prints all captured output of the task (but has to print stdout and stderr separately). So at the moment using RUST_TEST_NOCAPTURE=1 only makes sense when also using VERBOSE=1 which seems a little bit cumbersome.

Should I update the patch to only print the output of the tasks that actually execute the test (VERBOSE=1 includes other stuff, like the output of the task used to compile the test)? This will probably involve adding an extra flag to some functions in src/compiletest/runtest.rs to distinguish compilation runs from runs that execute the actual tests.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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

@fhahn
Copy link
Contributor Author

fhahn commented Feb 15, 2015

This patch also adds RUST_TEST_TASKS to the run environment, because that's another environment variable that could be interesting for run-pass tests that contain unit tests.

@brson
Copy link
Contributor

brson commented Feb 16, 2015

@bors: r+ ebe7f1 rollup.

Thanks! I think let's leave it requiring VERBOSE for now and see if it's obnoxious before adding more code to compiletest to sort it out.

@bors
Copy link
Contributor

bors commented Feb 16, 2015

🙀 You have the wrong number! Please try again with 9ebe7f1.

@brson
Copy link
Contributor

brson commented Feb 16, 2015

@bors: r+ 9ebe7f1 rollup

@Manishearth
Copy link
Member

This causes a failure of run-fail/test-tasks-invalid-value.rs (http://buildbot.rust-lang.org/builders/auto-linux-64-x-android-t/builds/3610/steps/test/logs/stdio)

(The test itself sets the RUST_TEST_TASKS env var, and this code seems to overwrite it)

@fhahn fhahn force-pushed the issue-17829-compiletest-nocapture branch from 9ebe7f1 to ef1308c Compare February 16, 2015 16:12
@fhahn
Copy link
Contributor Author

fhahn commented Feb 16, 2015

@ Manishearth thanks! I have updated the patch to only set RUST_TEST_TASK and RUST_TEST_NOCAPTURE if they were not defined in the test case already.

@Manishearth
Copy link
Member

@bors: r+ ef1308c rollup

bombless added a commit to bombless/rust that referenced this pull request Feb 23, 2015
…ture, r=Manishearth

This is a patch for rust-lang#17829.

In `compiletest` there are multiple layers which capture the output. The first layer is  `run_tests_console` which is used to execute all tests.

Then there are some tests that contain unit tests, which by default also captures output. Therefore `compiletest` adds `RUST_TEST_NOCAPTURE` (and `RUST_TEST_TASKS` for completeness) to the run environment of the task.

Finally, the task used to execute a test redirects stdout and stdin. At the moment, the `VERBOSE=1` prints all captured output of the task (but has to print stdout and stderr separately). So at the moment using `RUST_TEST_NOCAPTURE=1` only makes sense when also using `VERBOSE=1` which seems a little bit cumbersome.

Should I update the patch to only print the output of the tasks that actually execute the test (`VERBOSE=1` includes other stuff, like the output of the task used to compile the test)? This will probably involve adding an extra flag to some functions in `src/compiletest/runtest.rs` to distinguish compilation runs from runs that execute the actual tests.
Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 23, 2015
…ture, r=Manishearth

 This is a patch for rust-lang#17829.

In `compiletest` there are multiple layers which capture the output. The first layer is  `run_tests_console` which is used to execute all tests.

Then there are some tests that contain unit tests, which by default also captures output. Therefore `compiletest` adds `RUST_TEST_NOCAPTURE` (and `RUST_TEST_TASKS` for completeness) to the run environment of the task.

Finally, the task used to execute a test redirects stdout and stdin. At the moment, the `VERBOSE=1` prints all captured output of the task (but has to print stdout and stderr separately). So at the moment using `RUST_TEST_NOCAPTURE=1` only makes sense when also using `VERBOSE=1` which seems a little bit cumbersome.

Should I update the patch to only print the output of the tasks that actually execute the test (`VERBOSE=1` includes other stuff, like the output of the task used to compile the test)? This will probably involve adding an extra flag to some functions in `src/compiletest/runtest.rs` to distinguish compilation runs from runs that execute the actual tests.
@alexcrichton alexcrichton merged commit ef1308c into rust-lang:master Feb 23, 2015
@fhahn fhahn deleted the issue-17829-compiletest-nocapture branch March 1, 2015 17:34
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.

7 participants