test: migrate `help` to snapbox #14060

choznerol · 2024-06-13T15:56:26Z

What does this PR try to resolve?

Part of #14039.

Migrate tests/testsuite/help.rs to snapbox.

How should we test and review this PR?

I followed #14039. The #![allow(deprecated)] was removed and tests/testsuite/help.rs is still passing.

Additional information

I accidentally noticed that L155 would always pass despite the .with_stderr_data("whatever I write here"), either before or after this PR. It's probably related to run_expect_error(). Will investigate and open an issue. 👉 Filed #14076

rustbot · 2024-06-13T15:56:30Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @weihanglo (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

weihanglo · 2024-06-13T16:02:23Z

tests/testsuite/help.rs

    cargo_process("search --help")
-        .with_stdout_contains("[..] --frozen [..]")
+        .with_stdout_data(


You could use str![] and regenerate snapshots by running this command SNAPSHOTS=overwrite cargo t --test testsuite help::

Suggested change

.with_stdout_data(

.with_stdout_data(str![])

Thanks! Just get SNAPSHOTS=overwrite working and pushed 8db30bc.

The `with_stderr_contains()` (as well as the new `with_stderr_data()` too, see rust-lang#14060) has no effect when using with `run_expect_error()`.

epage · 2024-06-13T16:08:38Z

tests/testsuite/help.rs

    // Ensure that help output goes to stdout, not stderr.
-    cargo_process("search --help").with_stderr("").run();
+    cargo_process("search --help").with_stderr_data("").run();
    cargo_process("search --help")
-        .with_stdout_contains("[..] --frozen [..]")
+        .with_stdout_data(


We can probably delete this test because we have help UI tests for every command, like

https://github.com/rust-lang/cargo/blob/master/tests/testsuite/cargo_search/help/mod.rs

Thanks. Removed the search --help cases in eef057e.

P.s. I left L11-16 untouched, as the exact command are slightly different from the existing UI tests.

epage · 2024-06-13T16:09:26Z

tests/testsuite/help.rs

@@ -147,7 +151,7 @@ fn help_alias() {
    // The `empty-alias` returns an error.
    cargo_process("help empty-alias")
        .env("PATH", Path::new(""))
-        .with_stderr_contains("[..]The subcommand 'empty-alias' wasn't recognized[..]")
+        .with_stderr_data("[..] The subcommand 'empty-alias' wasn't recognized [..]")


Should we drop the [..]s? We are trying to move away from them where it makes sense because they get in the way of snapshot updating

~~Removed in e5e9f2f, but in fact there is another issue #14076 with this test case. I mentioned that as well in the same commit.~~

Fixed in 8db30bc

epage · 2024-06-13T16:10:45Z

tests/testsuite/help.rs

 fn alias_z_flag_help() {
    for cmd in ["build", "run", "check", "test", "b", "r", "c", "t"] {
        cargo_process(&format!("{cmd} -Z help"))
-            .with_stdout_contains(
-                "    -Z allow-features[..]  Allow *only* the listed unstable features",
+            .with_stdout_data(
+                "\
+...
+    -Z allow-features[..]  Allow *only* the listed unstable features
+...
+",
            )
            .run();


We also might be able to remove this as well

https://github.com/rust-lang/cargo/blob/master/tests/testsuite/cargo/z_help/stdout.term.svg

Thanks. Removed in 3fd3879.

…p::` # Conflicts: # tests/testsuite/help.rs

…uite help::`" This reverts commit cbaa4a89e60507366401ac0076d652f5eafc4998.

# Conflicts: # tests/testsuite/help.rs

weihanglo · 2024-06-15T13:22:36Z

tests/testsuite/help.rs

+        .with_stderr_data("The subcommand 'empty-alias' wasn't recognized
+
+FIXME: #14076 This assertion isn't working, as this line should have caused a test failure but didn't.
+        ",
+        )
        .run_expect_error();


Suggested change

.with_stderr_data("The subcommand 'empty-alias' wasn't recognized

FIXME: #14076 This assertion isn't working, as this line should have caused a test failure but didn't.

",

)

.run_expect_error();

.with_status(101)

.with_stderr_data(str![[r#"

[ERROR] no such command: `empty-alias`

Did you mean `empty-alias`?

View all installed commands with `cargo --list`

Find a package to install `empty-alias` with `cargo search cargo-empty-alias`

"#]])

.run();

Something like this should work. See #14076 (comment).

(just realized that the error message regressed…)

Would you mind posting another PR fixing #14076 and then update this afterward?

Thanks! Just get SNAPSHOTS=overwrite working and pushed 8db30bc.

Would you mind posting another PR fixing #14076 and then update this afterward?

Sure! Will address #14076 first with #14078, and then resolve the conflict here.

The `with_stderr_contains()` (as well as the new `with_stderr_data()` too, see rust-lang#14060) has no effect when using with `run_expect_error()`.

This reverts commit 18f4c3f.

The `with_stderr_contains()` (as well as the new `with_stderr_data()` too, see rust-lang#14060) has no effect when using with `run_expect_error()`.

choznerol · 2024-06-15T17:12:39Z

@rustbot review

weihanglo · 2024-06-15T21:24:41Z

Thanks!

@bors r+

bors · 2024-06-15T21:24:44Z

📌 Commit 8db30bc has been approved by weihanglo

It is now in the queue for this repository.

bors · 2024-06-15T21:25:51Z

⌛ Testing commit 8db30bc with merge ea16f96...

bors · 2024-06-15T21:55:11Z

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

The `with_stderr_contains()` (as well as the new `with_stderr_data()` too, see rust-lang#14060) has no effect when using with `run_expect_error()`.

Update cargo 13 commits in a1f47ec3f7cd076986f1bfcd7061f2e8cb1a726e..3ed207e416fb2f678a40cc79c02dcf4f936a21ce 2024-06-15 01:10:07 +0000 to 2024-06-18 19:18:22 +0000 - test: prefer raw string for regex reduction (rust-lang/cargo#14099) - test: migrate tree and tree_graph_features to snapbox (rust-lang/cargo#14094) - test: Migrate some files to snapbox (rust-lang/cargo#14069) - remove some legacy public dependency code from the resolver (rust-lang/cargo#14090) - fix(fix): Address problems with implicit -> explicit feature migration (rust-lang/cargo#14018) - refactor: 1.79 cleanup (rust-lang/cargo#14088) - test: migrate `git_(gc|shallow)` to snapbox (rust-lang/cargo#14087) - test: migrate timings_works to snapbox (rust-lang/cargo#14082) - test: migrate minimal_versions to snapbox (rust-lang/cargo#14080) - Remove `run_expect_error` to avoid tests incorrectly passing (rust-lang/cargo#14078) - test: migrate help to snapbox (rust-lang/cargo#14060) - test: Migrate tests/testsuite/co*.rs to snapbox (rust-lang/cargo#14079) - Use `std::fs::absolute` instead of reimplementing it (rust-lang/cargo#14075)

test: migrate help to snapbox

7c28d80

rustbot assigned weihanglo Jun 13, 2024

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 13, 2024

weihanglo reviewed Jun 13, 2024

View reviewed changes

epage reviewed Jun 13, 2024

View reviewed changes

choznerol added 4 commits June 15, 2024 15:02

Tmp commit prior to `SNAPSHOTS=overwrite cargo t --test testsuite hel…

715cba9

…p::` # Conflicts: # tests/testsuite/help.rs

Revert "Tmp commit prior to `SNAPSHOTS=overwrite cargo t --test tests…

3a95c78

…uite help::`" This reverts commit cbaa4a89e60507366401ac0076d652f5eafc4998.

Dedup with cargo::z_help::case (rust-lang#14060 (comment))

3fd3879

# Conflicts: # tests/testsuite/help.rs

Dedup with cargo_search::help::case (rust-lang#14060 (comment))

eef057e

choznerol mentioned this pull request Jun 15, 2024

test: with_stderr_(data|contains) has no effect when chained with run_expect_error() #14076

Closed

Remove [..] and mention rust-lang#14076 caveat

e5e9f2f

choznerol marked this pull request as ready for review June 15, 2024 07:49

choznerol requested review from epage and weihanglo June 15, 2024 07:56

weihanglo reviewed Jun 15, 2024

View reviewed changes

Use snapshot generated by SNAPSHOTS=overwrite cargo test

8db30bc

choznerol added a commit to choznerol/cargo that referenced this pull request Jun 15, 2024

Reproduce rust-lang#14060

36b44ed

The `with_stderr_contains()` (as well as the new `with_stderr_data()` too, see rust-lang#14060) has no effect when using with `run_expect_error()`.

choznerol added a commit to choznerol/cargo that referenced this pull request Jun 15, 2024

Remove Execs::run_expect_error to avoid rust-lang#14060

0a20aab

choznerol added a commit to choznerol/cargo that referenced this pull request Jun 15, 2024

Revert "Reproduce rust-lang#14060"

e6c3692

This reverts commit 18f4c3f.

choznerol added a commit to choznerol/cargo that referenced this pull request Jun 15, 2024

Fix regression since rust-lang#14060 is now resolved

aa2fee8

choznerol added a commit to choznerol/cargo that referenced this pull request Jun 15, 2024

Reproduce rust-lang#14076

bc5d57c

The `with_stderr_contains()` (as well as the new `with_stderr_data()` too, see rust-lang#14060) has no effect when using with `run_expect_error()`.

weihanglo approved these changes Jun 15, 2024

View reviewed changes

bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 15, 2024

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 15, 2024

bors merged commit ea16f96 into rust-lang:master Jun 15, 2024
22 checks passed

choznerol added a commit to choznerol/cargo that referenced this pull request Jun 16, 2024

Reproduce rust-lang#14076

afe9d17

The `with_stderr_contains()` (as well as the new `with_stderr_data()` too, see rust-lang#14060) has no effect when using with `run_expect_error()`.

This was referenced Jun 16, 2024

Port from bespoke assertions of snapbox #14039

Open

Update cargo rust-lang/rust#126657

Merged

rustbot added this to the 1.81.0 milestone Jun 19, 2024

choznerol changed the title ~~test: migrate help to snapbox~~ test: migrate help to snapbox Jun 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: migrate `help` to snapbox #14060

test: migrate `help` to snapbox #14060

choznerol commented Jun 13, 2024 •

edited

Loading

rustbot commented Jun 13, 2024

weihanglo Jun 13, 2024

This comment was marked as outdated.

This comment was marked as outdated.

choznerol Jun 15, 2024

epage Jun 13, 2024

choznerol Jun 15, 2024 •

edited

Loading

epage Jun 13, 2024

choznerol Jun 15, 2024 •

edited

Loading

epage Jun 13, 2024

choznerol Jun 15, 2024

weihanglo Jun 15, 2024

weihanglo Jun 15, 2024

choznerol Jun 15, 2024

choznerol commented Jun 15, 2024

weihanglo commented Jun 15, 2024

bors commented Jun 15, 2024

bors commented Jun 15, 2024

bors commented Jun 15, 2024

-        .with_stderr_data("The subcommand 'empty-alias' wasn't recognized
-FIXME: #14076 This assertion isn't working, as this line should have caused a test failure but didn't.
-        ",
-        )
-        .run_expect_error();
+        .with_status(101)
+        .with_stderr_data(str![[r#"
+[ERROR] no such command: `empty-alias`
+	Did you mean `empty-alias`?
+	View all installed commands with `cargo --list`
+	Find a package to install `empty-alias` with `cargo search cargo-empty-alias`
+"#]])
+        .run();

test: migrate help to snapbox #14060

test: migrate help to snapbox #14060

Conversation

choznerol commented Jun 13, 2024 • edited Loading

What does this PR try to resolve?

How should we test and review this PR?

Additional information

rustbot commented Jun 13, 2024

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

choznerol Jun 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

choznerol Jun 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

choznerol commented Jun 15, 2024

weihanglo commented Jun 15, 2024

bors commented Jun 15, 2024

bors commented Jun 15, 2024

bors commented Jun 15, 2024

test: migrate `help` to snapbox #14060

test: migrate `help` to snapbox #14060

choznerol commented Jun 13, 2024 •

edited

Loading

choznerol Jun 15, 2024 •

edited

Loading

choznerol Jun 15, 2024 •

edited

Loading