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.

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

Cargo test quicker by not building untested examples when filtered #6683

Merged
merged 4 commits into from
Feb 25, 2019
Merged

Cargo test quicker by not building untested examples when filtered #6683

merged 4 commits into from
Feb 25, 2019

Conversation

dwijnand
Copy link
Member

Alternative to #6677
Fixes #6675
r? @ehuss

@dwijnand dwijnand force-pushed the cargo-test-quicker-by-not-building-untested-examples-when-filtered branch 2 times, most recently from 43ed4c3 to c816d06 Compare February 19, 2019 22:17
@bors
Copy link
Collaborator

bors commented Feb 20, 2019

☔ The latest upstream changes (presumably #6687) made this pull request unmergeable. Please resolve the merge conflicts.

@dwijnand
Move build tests to build
7d53e48
@dwijnand
Introduce CompileFilter::from_raw_arguments
12307fc 
... in order to give CompileFilter::new as a better API.
@dwijnand dwijnand force-pushed the cargo-test-quicker-by-not-building-untested-examples-when-filtered branch from c816d06 to 0917c01 Compare February 20, 2019 12:42
@ehuss
Copy link
Contributor

ehuss commented Feb 20, 2019

This seems to reduce what is tested for any argument, which I think will be a problem. For example, it's not uncommon to have cargo test -- --ignored or cargo test -- --test-threads 1 in CI. Perhaps don't change behavior if there is anything after --, and only if a bare TESTNAME is given?

Also, why choose the default of --lib --tests? Why not --lib --bins --tests? Or some other combination? If there is a unittest in a bin, then cargo test foo will no longer work.

@dwijnand
Copy link
Member Author

This seems to reduce what is tested for any argument, which I think will be a problem. For example, it's not uncommon to have cargo test -- --ignored or cargo test -- --test-threads 1 in CI. Perhaps don't change behavior if there is anything after --, and only if a bare TESTNAME is given?

Sounds good to me.

Also, why choose the default of --lib --tests? Why not --lib --bins --tests? Or some other combination? If there is a unittest in a bin, then cargo test foo will no longer work.

I assumed unit tests in a binary were as randomly-infrequent as unit tests in examples, but I see now it might not be as uncommon as I thought. I'll re-include binaries.

@dwijnand dwijnand force-pushed the cargo-test-quicker-by-not-building-untested-examples-when-filtered branch from 0917c01 to a6772a6 Compare February 20, 2019 19:48
@dwijnand
Introduce LibRule::Default to avoid failure mode
aef99e0 
The other targets have a no-failure FilterRule::All, but if lib is true
and the crate doesn't have a library, then generate_targets fails hard.
Let's give library a flexible option.
@dwijnand
Cargo test quicker by not building untested examples when filtered
110c813
@dwijnand dwijnand force-pushed the cargo-test-quicker-by-not-building-untested-examples-when-filtered branch from a6772a6 to 110c813 Compare February 25, 2019 15:14
@ehuss
Copy link
Contributor

ehuss commented Feb 25, 2019

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 25, 2019

📌 Commit 110c813 has been approved by ehuss

@bors
Copy link
Collaborator

bors commented Feb 25, 2019

⌛ Testing commit 110c813 with merge 4536bc9...

bors added a commit that referenced this pull request Feb 25, 2019
@bors
Auto merge of #6683 - dwijnand:cargo-test-quicker-by-not-building-unt…
4536bc9 
…ested-examples-when-filtered, r=ehuss

Cargo test quicker by not building untested examples when filtered

Alternative to #6677
Fixes #6675
r? @ehuss
@bors
Copy link
Collaborator

bors commented Feb 25, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: ehuss
Pushing 4536bc9 to master...

@bors bors merged commit 110c813 into rust-lang:master Feb 25, 2019
@bors bors mentioned this pull request Feb 25, 2019
Closed
@dwijnand dwijnand deleted the cargo-test-quicker-by-not-building-untested-examples-when-filtered branch February 25, 2019 22:07
@ehuss ehuss mentioned this pull request Mar 4, 2019
Merged
bors added a commit to rust-lang/rls that referenced this pull request Mar 4, 2019
@bors
Auto merge of #1388 - ehuss:update-cargo, r=Xanewok
6a1b5a9 
Update cargo and clippy.

Cargo had a minor api change due to rust-lang/cargo#6683.

Clippy needed an update to build with latest nightly.
@ehuss ehuss mentioned this pull request Mar 4, 2019
Merged
bors added a commit to rust-lang/rust that referenced this pull request Mar 4, 2019
@bors
Auto merge of #58920 - ehuss:update-cargo-rls, r=alexcrichton
80bf632 
Update cargo, rls

## cargo

6 commits in 5c6aa46e6f28661270979696e7b4c2f0dff8628f..716b02cb4c7b75ce435eb06defa25bc2d725909c
2019-02-22 19:32:35 +0000 to 2019-03-02 14:23:51 +0000
- Some test/bench-related tweaks (rust-lang/cargo#6707)
- Fix links to the permanent home of the edition guide. (rust-lang/cargo#6703)
- HTTPS all the things (rust-lang/cargo#6614)
- Cargo test quicker by not building untested examples when filtered (rust-lang/cargo#6683)
- Link from ARCHITECTURE.md to docs.rs and github (rust-lang/cargo#6695)
- Update how to install rustfmt (rust-lang/cargo#6696)

## rls

9 commits in 0d6f53e1a4adbaf7d83cdc0cb54720203fcb522e..6a1b5a9cfda2ae19372e0613e76ebefba36edcf5
2019-02-14 07:52:15 +0000 to 2019-03-04 20:24:45 +0000
- Update cargo and clippy. (rust-lang/rls#1388)
- catch up rust-lang/rust PR#58321 (rust-lang/rls#1384)
- Apply Clippy fixes (rust-lang/rls#1327)
- Various cosmetic improvements (rust-lang/rls#1326)
- Make test `RlsHandle` transport-agnostic (rust-lang/rls#1317)
- Translate remaining tests (rust-lang/rls#1320)
- Remove unnecessary #![feature]s (rust-lang/rls#1323)
- Update Clippy (rust-lang/rls#1319)
- Update Clippy (rust-lang/rls#1315)

cc @Xanewok
bors added a commit to rust-lang/rust that referenced this pull request Mar 9, 2019
@bors
Auto merge of #58920 - ehuss:update-cargo-rls, r=ehuss
777cc25 
Update cargo, rls, books

## cargo

10 commits in 5c6aa46e6f28661270979696e7b4c2f0dff8628f..95b45eca19ac785263fed98ecefe540bb47337ac
2019-02-22 19:32:35 +0000 to 2019-03-06 19:24:30 +0000
- Relax some rustdoc tests. (rust-lang/cargo#6721)
- Include build script execution in the fingerprint. (rust-lang/cargo#6720)
- part of the infrastructure for public & private dependencies in the resolver (rust-lang/cargo#6653)
- Bump to 0.36.0 (rust-lang/cargo#6718)
- Some test/bench-related tweaks (rust-lang/cargo#6707)
- Fix links to the permanent home of the edition guide. (rust-lang/cargo#6703)
- HTTPS all the things (rust-lang/cargo#6614)
- Cargo test quicker by not building untested examples when filtered (rust-lang/cargo#6683)
- Link from ARCHITECTURE.md to docs.rs and github (rust-lang/cargo#6695)
- Update how to install rustfmt (rust-lang/cargo#6696)

## rls

9 commits in 0d6f53e1a4adbaf7d83cdc0cb54720203fcb522e..6a1b5a9cfda2ae19372e0613e76ebefba36edcf5
2019-02-14 07:52:15 +0000 to 2019-03-04 20:24:45 +0000
- Update cargo and clippy. (rust-lang/rls#1388)
- catch up rust-lang/rust PR#58321 (rust-lang/rls#1384)
- Apply Clippy fixes (rust-lang/rls#1327)
- Various cosmetic improvements (rust-lang/rls#1326)
- Make test `RlsHandle` transport-agnostic (rust-lang/rls#1317)
- Translate remaining tests (rust-lang/rls#1320)
- Remove unnecessary #![feature]s (rust-lang/rls#1323)
- Update Clippy (rust-lang/rls#1319)
- Update Clippy (rust-lang/rls#1315)

cc @Xanewok

## Books
See #58936 for details.
@ehuss ehuss mentioned this pull request May 27, 2019
Open
@weihanglo weihanglo mentioned this pull request Jan 7, 2022
Closed
@weihanglo weihanglo mentioned this pull request Jan 18, 2022
Merged
bors added a commit that referenced this pull request Jan 20, 2022
@bors
Auto merge of #10305 - weihanglo:issue-10268, r=alexcrichton
58790d3 
do not compile test for bins flagged as `test = false`

### What does this PR try to resolve?

Fixes #10268

#6683 introduced a behavior that compiles all bin targets, but for bins with `test = false` they shouldn't be compiled with `--test` as testbins.

### How should we test and review this PR?

In the first commit of this PR, I refines the test `test_filtered_excludes_compiling_examples` to reflect the current wrong behavior (test passed). The following two commits correct the behavior and the test accordingly. The last few commits encapsulate scattered target selection logic into functions on `CompileFilter`.
@ehuss ehuss added this to the 1.35.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ehuss ehuss

Labels
None yet
Projects
None yet
Milestone
1.35.0
Development

Successfully merging this pull request may close these issues.

When running cargo test, cargo also re-builds all examples
3 participants
@dwijnand @bors @ehuss