fix(test): Distinguish 'testname' from escaped arguments

[epage]

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

[rust-highfive]

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

[weihanglo]

Sorry for my oversight on this behaviour change.

Could you help double confirm that cargo run, cargo rustc, and cargo rustdoc are not affected? From my understanding they should behave as before and just tested by hands them seem fine.

[weihanglo]

Feel free to r=weihanglo once you've done the confirmation. I may update Cargo in rust-lang/rust after this get merged.

[epage]

Could you help double confirm that cargo run, cargo rustc, and cargo rustdoc are not affected? From my understanding they should behave as before and just tested by hands them seem fine.

Those commands are unrelated to the problem. This is a problem specifically with how I handled the change from .last(true).trailing_var_arg(true) which only affected cargo bench and cargo test (I searched #11159 multiple times to confirm). Even then, the only way to observe a problem is if people are relying on the specialization of logic that exists only in cargo test.. This precludes observing a change in behavior in cargo bench as it just takes BENCHNAME and prefixes it to args without any logic conditioned on the existence of BENCHNAME; no matter whether -- is used or not, the same behavior will occur. It took me several tries to even create a good test case for cargo test that clearly showed the problem due to how specific the relevant problem is.

Sorry for my oversight on this behaviour change.

Responsibility first lies with the author, reviewers can't be expected to catch everything. Unless you knew to check this exact case, I bet we would have missed it anyways.

r=weihanglo

[epage]

@bors r=weihanglo

(of course I forgot to tag bors)

[bors]

📌 Commit 3dd8413 has been approved by weihanglo

It is now in the queue for this repository.

[bors]

⌛ Testing commit 3dd8413 with merge 3cdf1ab...

[weihanglo]

I should have thought of that, since I did some fixes around it…

Anyway, thanks for your quick patch!

[CAD97]

(hi I'm the one that found the behavior change)

It's definitely good to restore previous behavior here, but it's also I think worth discussing whether cargo test TESTARG and cargo test -- TESTARG should be behaving differently.

To me, it seems quite odd that being explicit about which arguments are passed to the test runner should change how the test runner is run.

Plus, in general, due to how rustdoc (ab)uses the test runner, flags can have distinctly different behavior. (The notable one I'm dealing with is --ignored.)

[epage]

(hi I'm the one that found the behavior change)

It's definitely good to restore previous behavior here, but it's also I think worth discussing whether cargo test TESTARG and cargo test -- TESTARG should be behaving differently.

To me, it seems quite odd that being explicit about which arguments are passed to the test runner should change how the test runner is run.

Plus, in general, due to how rustdoc (ab)uses the test runner, flags can have distinctly different behavior. (The notable one I'm dealing with is --ignored.)

#6981 is the issue for that.

[bors]

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 3cdf1ab to master...