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

Relaxes expectation of cargo test tests to accept test execution time #8884

Merged
merged 1 commit into from
Nov 23, 2020
Merged

Relaxes expectation of cargo test tests to accept test execution time #8884

merged 1 commit into from
Nov 23, 2020

Conversation

jakoschiko
Copy link
Contributor

rust-lang/rust#75752 changes the output of libtest.

For example, output before:

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

Output after:

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

This breaks some cargo tests: https://github.com/rust-lang-ci/rust/runs/1439245145

As a preparation for the merge, this PR relaxes the test expectations of cargo test tests to accept both outputs. I'm using the existing pattern feature of Execs::with_stdout:

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out[..]

I made this change for the following tests:

  • test::can_not_mix_doc_tests_and_regular_tests
  • test::cargo_test_quiet_with_harness
  • test::test_filtered_excludes_compiling_examples
  • test::cargo_test_doctest_xcompile_ignores (didn't fail in Rollup of 7 pull requests rust#79317, but failed locally)
  • test::cargo_test_doctest_xcompile (doesn't run locally, I changed it just to be safe)
  • test::cargo_test_doctest_xcompile_runner (doesn't run locally, I changed it just to be safe)
  • test::cargo_test_doctest_xcompile_no_runner (doesn't run locally, I changed it just to be safe)

If requested, I will open another PR later to change the expectation to:

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in [..]s

However, I don't know how to handle WASM targets because WASM doesn't support time measuring, therefore the libtest output didn't change for WASM. Is WASM even relevant here?

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @ehuss (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 22, 2020
@alexcrichton
Copy link
Member

@bors: r+

Thanks!

@bors
Copy link
Contributor

bors commented Nov 23, 2020

📌 Commit 62ddaf3 has been approved by alexcrichton

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 23, 2020
@bors
Copy link
Contributor

bors commented Nov 23, 2020

⌛ Testing commit 62ddaf3 with merge 41c0f48...

@bors
Copy link
Contributor

bors commented Nov 23, 2020

☀️ Test successful - checks-actions
Approved by: alexcrichton
Pushing 41c0f48 to master...

@bors bors merged commit 41c0f48 into rust-lang:master Nov 23, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 24, 2020
Update cargo

10 commits in 2af662e22177a839763ac8fb70d245a680b15214..bfca1cd22bf514d5f2b6c1089b0ded0ba7dfaa6e
2020-11-12 19:04:56 +0000 to 2020-11-24 16:33:21 +0000
- Shrink the progress bar, to give more space after it. (rust-lang/cargo#8892)
- Add some comments to the toml code (rust-lang/cargo#8887)
- Start searching git config at new path (rust-lang/cargo#8886)
- Fix documentation for CARGO_PRIMARY_PACKAGE. (rust-lang/cargo#8891)
- Bump to 0.51.0, update changelog (rust-lang/cargo#8894)
- Publish target's "doc" setting when emitting metadata (rust-lang/cargo#8869)
- Relaxes expectation of `cargo test` tests to accept test execution time (rust-lang/cargo#8884)
- Finish implementation of `-Zextra-link-arg`. (rust-lang/cargo#8441)
- Reproducible crate builds (rust-lang/cargo#8864)
- Allow resolver="1" to explicitly use the old resolver behavior. (rust-lang/cargo#8857)
ehuss pushed a commit to ehuss/cargo that referenced this pull request Nov 30, 2020
… r=alexcrichton

Relaxes expectation of `cargo test` tests to accept test execution time

rust-lang/rust#75752 changes the output of libtest.

For example, output before:

```
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
```

Output after:

```
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s
```

This breaks some cargo tests: https://github.com/rust-lang-ci/rust/runs/1439245145

As a preparation for the merge, this PR relaxes the test expectations of `cargo test` tests to accept both outputs. I'm using the existing pattern feature of `Execs::with_stdout`:

```
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out[..]
```

I made this change for the following tests:

- `test::can_not_mix_doc_tests_and_regular_tests`
- `test::cargo_test_quiet_with_harness`
- `test::test_filtered_excludes_compiling_examples`
- `test::cargo_test_doctest_xcompile_ignores` (didn't fail in rust-lang/rust#79317, but failed locally)
- `test::cargo_test_doctest_xcompile` (doesn't run locally, I changed it just to be safe)
- `test::cargo_test_doctest_xcompile_runner` (doesn't run locally, I changed it just to be safe)
- `test::cargo_test_doctest_xcompile_no_runner` (doesn't run locally, I changed it just to be safe)

If requested, I will open another PR later to change the expectation to:

```
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in [..]s
```

However, I don't know how to handle WASM targets because WASM doesn't support time measuring, therefore the libtest output didn't change for WASM. Is WASM even relevant here?
@ehuss ehuss added this to the 1.50.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants