Skip to content

Commit

Permalink
Auto merge of #11190 - epage:test, r=weihanglo
Browse files Browse the repository at this point in the history
fix(test): Distinguish 'testname' from escaped arguments

When working on clap v4, it appeared that `last` and `trailing_var_arg`
are mutually exclusive, so I called that out in the debug asserts in
#4187.  Unfortunately, I didn't document my research on this
as my focus was elsewhere.

When updating cargo to use clap v4 in #11159, I found `last` and
`trailing_var_arg` were used together.  I figured this was what I was
trying to catch with above and I went off of my intuitive sense of these
commands and assumed `trailing_var_arg` was intended, not knowing about
the `testname` positional that is mostly just a forward to `libtest`,
there for documentation purposes, except for a small optimization.

So it looks like we just need the `last` call and not the
`trailing_var_arg` call.

This restores us to behavior from 531ce13 which is what I originally
wrote the tests against.

It looks like #11159 was merged after the last beta branch was made, so we shouldn't
need to cherry-pick this into any other release.

For reviewing this, I made the test updates in the first commit, showing the wrong behavior.  The following commit fixes the behavior and updates the tests to expected behavior.

Fixes #11191
  • Loading branch information
bors committed Oct 7, 2022
2 parents aff3bb8 + 3dd8413 commit 3cdf1ab
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub fn cli() -> Command {
Arg::new("args")
.help("Arguments for the bench binary")
.num_args(0..)
.trailing_var_arg(true),
.last(true),
)
.arg_targets_all(
"Benchmark only this package's library",
Expand Down
2 changes: 1 addition & 1 deletion src/bin/cargo/commands/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub fn cli() -> Command {
Arg::new("args")
.help("Arguments for the test binary")
.num_args(0..)
.trailing_var_arg(true),
.last(true),
)
.arg(
flag(
Expand Down
110 changes: 105 additions & 5 deletions tests/testsuite/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -725,13 +725,101 @@ fn dont_run_examples() {
}

#[cargo_test]
fn pass_through_command_line() {
fn pass_through_escaped() {
let p = project()
.file(
"src/lib.rs",
"
#[test] fn foo() {}
#[test] fn bar() {}
/// ```rust
/// assert!(foo::foo());
/// ```
pub fn foo() -> bool {
true
}
/// ```rust
/// assert!(!foo::bar());
/// ```
pub fn bar() -> bool {
false
}
#[test] fn test_foo() {
assert!(foo());
}
#[test] fn test_bar() {
assert!(!bar());
}
",
)
.build();

p.cargo("test -- bar")
.with_stderr(
"\
[COMPILING] foo v0.0.1 ([CWD])
[FINISHED] test [unoptimized + debuginfo] target(s) in [..]
[RUNNING] [..] (target/debug/deps/foo-[..][EXE])
[DOCTEST] foo
",
)
.with_stdout_contains("running 1 test")
.with_stdout_contains("test test_bar ... ok")
.run();

p.cargo("test -- foo")
.with_stderr(
"\
[FINISHED] test [unoptimized + debuginfo] target(s) in [..]
[RUNNING] [..] (target/debug/deps/foo-[..][EXE])
[DOCTEST] foo
",
)
.with_stdout_contains("running 1 test")
.with_stdout_contains("test test_foo ... ok")
.run();

p.cargo("test -- foo bar")
.with_stderr(
"\
[FINISHED] test [unoptimized + debuginfo] target(s) in [..]
[RUNNING] [..] (target/debug/deps/foo-[..][EXE])
[DOCTEST] foo
",
)
.with_stdout_contains("running 2 tests")
.with_stdout_contains("test test_foo ... ok")
.with_stdout_contains("test test_bar ... ok")
.run();
}

// Unlike `pass_through_escaped`, doctests won't run when using `testname` as an optimization
#[cargo_test]
fn pass_through_testname() {
let p = project()
.file(
"src/lib.rs",
"
/// ```rust
/// assert!(foo::foo());
/// ```
pub fn foo() -> bool {
true
}
/// ```rust
/// assert!(!foo::bar());
/// ```
pub fn bar() -> bool {
false
}
#[test] fn test_foo() {
assert!(foo());
}
#[test] fn test_bar() {
assert!(!bar());
}
",
)
.build();
Expand All @@ -745,7 +833,7 @@ fn pass_through_command_line() {
",
)
.with_stdout_contains("running 1 test")
.with_stdout_contains("test bar ... ok")
.with_stdout_contains("test test_bar ... ok")
.run();

p.cargo("test foo")
Expand All @@ -756,7 +844,19 @@ fn pass_through_command_line() {
",
)
.with_stdout_contains("running 1 test")
.with_stdout_contains("test foo ... ok")
.with_stdout_contains("test test_foo ... ok")
.run();

p.cargo("test foo -- bar")
.with_stderr(
"\
[FINISHED] test [unoptimized + debuginfo] target(s) in [..]
[RUNNING] [..] (target/debug/deps/foo-[..][EXE])
",
)
.with_stdout_contains("running 2 tests")
.with_stdout_contains("test test_foo ... ok")
.with_stdout_contains("test test_bar ... ok")
.run();
}

Expand Down

0 comments on commit 3cdf1ab

Please sign in to comment.