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

test: Migrate some files to snapbox #14069

Merged
merged 3 commits into from
Jun 18, 2024
Merged

Conversation

heisen-li
Copy link
Contributor

What does this PR try to resolve?

This PR addresses the migration of the following files:

tests/testsuite/build_script_extra_link_arg.rs
tests/testsuite/cache_lock.rs
tests/testsuite/cache_messages.rs

part of #14039

@rustbot
Copy link
Collaborator

rustbot commented Jun 14, 2024

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 14, 2024
tests/testsuite/build_script_extra_link_arg.rs Outdated Show resolved Hide resolved
[FINISHED] `test` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
[DOCTEST] foo
[RUNNING] `rustdoc [..]--crate-name foo [..]-C link-arg=--this-is-a-bogus-flag[..]
[ERROR] doctest failed, to rerun pass `--doc`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI failed, but my local was a success.

tests/testsuite/build_script_extra_link_arg.rs Outdated Show resolved Hide resolved
",
expected
))
.run();
}

#[allow(deprecated)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On account ofwith_stderr_does_not_contain(),added #[allow(deprecated)].

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we can just assert stdout with str![] to replace with_stderr_does_not_contain here. They should be all empty except the very first invocation.

And we can add str! for each with_stderr_data.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. The first one looks empty too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry. FOund that I was wrong 🤦🏾‍♂️. WRAPPER CALLED: might be matched by ... wild card. We should keep with_stderr_does_not_contain. Maybe make the literal shorter so it won't accidentally mismatch in the future? Like with_stderr_does_not_contain("WRAPPER CALLED: [..])?

.with_stdout_does_not_contain(
"WRAPPER CALLED: rustc --crate-name foo --edition=2015 src/lib.rs [..]",
)
.run();
}

#[allow(deprecated)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On account ofwith_stderr_does_not_contain(),added #[allow(deprecated)].

tests/testsuite/build_script_extra_link_arg.rs Outdated Show resolved Hide resolved
tests/testsuite/build_script_extra_link_arg.rs Outdated Show resolved Hide resolved
@@ -160,7 +159,17 @@ fn clears_cache_after_fix() {
// Make sure the cache is invalidated when there is no output.
let p = project().file("src/lib.rs", "fn asdf() {}").build();
// Fill the cache.
p.cargo("check").with_stderr_contains("[..]asdf[..]").run();
p.cargo("check")
.with_stderr_data(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While rustc invocation is omitted, I believe we can still wrap it with str![].

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it has been modified.

BTW: Sorry, I'm a bit confused now, what are the criteria for using or not using str![]?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #14039 "4. Resolve non-literal deprecations".

To me, I try porting all of the with str![] unless it is too specific, fragile, or hard to model with str![].

tests/testsuite/cache_messages.rs Outdated Show resolved Hide resolved
tests/testsuite/cache_messages.rs Show resolved Hide resolved
tests/testsuite/cache_messages.rs Outdated Show resolved Hide resolved
tests/testsuite/cache_messages.rs Outdated Show resolved Hide resolved
tests/testsuite/cache_messages.rs Outdated Show resolved Hide resolved
@heisen-li heisen-li force-pushed the build_script_extra branch 2 times, most recently from 05cfed0 to b0f4c94 Compare June 17, 2024 11:54
tests/testsuite/cache_messages.rs Outdated Show resolved Hide resolved
"\
[FRESH] foo v0.0.1 ([ROOT]/foo)
WRAPPER CALLED: rustc [..]
...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not necessary and can be wrapped with str![]

Suggested change
...

",
expected
))
.run();
}

#[allow(deprecated)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we can just assert stdout with str![] to replace with_stderr_does_not_contain here. They should be all empty except the very first invocation.

And we can add str! for each with_stderr_data.

@weihanglo
Copy link
Member

Thanks. Looks good.

@bors r+

@bors
Copy link
Contributor

bors commented Jun 18, 2024

📌 Commit 2e1f7b4 has been approved by weihanglo

It is now in the queue for this repository.

@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 Jun 18, 2024
@bors
Copy link
Contributor

bors commented Jun 18, 2024

⌛ Testing commit 2e1f7b4 with merge b2739ba...

@bors
Copy link
Contributor

bors commented Jun 18, 2024

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

@bors bors merged commit b2739ba into rust-lang:master Jun 18, 2024
22 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 19, 2024
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)

<!--
r? ghost
-->
@rustbot rustbot added this to the 1.81.0 milestone Jun 19, 2024
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.

6 participants