test: migrate registry and registry_auth to snapbox#14149
test: migrate registry and registry_auth to snapbox#14149bors merged 2 commits intorust-lang:masterfrom
Conversation
|
r? @weihanglo rustbot has assigned @weihanglo. Use |
fdcd693 to
c259776
Compare
weihanglo
left a comment
There was a problem hiding this comment.
Apart from those two comments, LGTM
tests/testsuite/registry_auth.rs
Outdated
|
|
||
| let p = make_project(); | ||
| cargo(&p, "build").with_stderr(SUCCESS_OUTPUT).run(); | ||
| cargo(&p, "build").with_stderr_data(SUCCESS_OUTPUT).run(); |
There was a problem hiding this comment.
Is it possible to make SUCCESS_OUTPUT snapshot aware?
There was a problem hiding this comment.
I feel that this comment contradict to #14149 (comment).
There was a problem hiding this comment.
Is it possible to make
SUCCESS_OUTPUTsnapshot aware?
Is this referring to using its own str![] instead of the same SUCCESS_OUTPUT?
There was a problem hiding this comment.
Yes. The goal is making assertions auto-updatable.
tests/testsuite/registry.rs
Outdated
| } | ||
|
|
||
| fn simple() { | ||
| fn simple(is_http: bool) { |
There was a problem hiding this comment.
I feel it a bit confusing. By just looking at code, I don't know why this is_http is needed. Apparently there is not different between each branch. Is there a way to document it, or this should be fixed in snapbox or go the other way round?
There was a problem hiding this comment.
Currently, if a function contains str![] and is called twice, the content will become something like str![[...] [...]].
This PR introduces an argument is_http to handle str![] separately, allowing it to be filled with the correct content (even though the current testsuite doesn't show the difference).
If appending multiple times is not the expected behavior, I think this should be fixed in snapbox.
Alternatively, it's possible to eliminate the need for the is_http argument by avoiding str![] altogether.
There was a problem hiding this comment.
I think tests/testsuite/list_availables.rs is also worth mentioning here. It reuses a function for assertions with distinct commands multiple times.
There was a problem hiding this comment.
Thanks for the reply!
I still found it cumbersome to understand. Maybe we should parameterize over inline snapshot itself. That may look like
fn simple_git() {
simple(str![[r#"
[UPDATING] `dummy-registry` index
[LOCKING] 2 packages to latest compatible versions
[DOWNLOADING] crates ...
[DOWNLOADED] bar v0.0.1 (registry `dummy-registry`)
[CHECKING] bar v0.0.1
[CHECKING] foo v0.0.1 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
"#]], str[[r#"
[CHECKING] bar v0.0.1
[CHECKING] foo v0.0.1 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
"#]]);
}(Note that we have two shared assertions hence two arguments)
What do you think?
There was a problem hiding this comment.
Yeah, this also make sense to me! Thanks!
c259776 to
b7c0054
Compare
|
All feedback addressed! |
|
Filed assert-rs/snapbox#345 though I don't think this is blocked on it. I appreciate the effort you put on this back-and-forth snapshot changes. Thanks a lot! @bors r+ |
|
☀️ Test successful - checks-actions |
Update cargo 23 commits in 4ed7bee47f7dd4416b36fada1909e9a62c546246..a515d463427b3912ec0365d106791f88c1c14e1b 2024-06-25 16:28:22 +0000 to 2024-07-02 20:53:36 +0000 - test: migrate rust_version and rustc* to snapbox (rust-lang/cargo#14177) - test: mirgate fix* and future_incompat_report to snapbox (rust-lang/cargo#14173) - test：migrate `edition/error` to snapbox (rust-lang/cargo#14175) - chore(deps): update compatible (rust-lang/cargo#14174) - refactor(source): Clean up after PathSource/RecursivePathSource split (rust-lang/cargo#14169) - test: Migrate some files to snapbox (rust-lang/cargo#14132) - test: fix several assertions (rust-lang/cargo#14167) - test: replace glob with explicit unordered calls (rust-lang/cargo#14166) - Make it clear that `CARGO_CFG_TARGET_FAMILY` is multi-valued (rust-lang/cargo#14165) - Document `CARGO_CFG_TARGET_ABI` (rust-lang/cargo#14164) - test: Migrate git to snapbox (rust-lang/cargo#14159) - test: migrate some files to snapbox (rust-lang/cargo#14158) - test: migrate registry and registry_auth to snapbox (rust-lang/cargo#14149) - gix: remove `revision` feature from cargo (rust-lang/cargo#14160) - test: migrate package* and publish* to snapbox (rust-lang/cargo#14130) - More `update --breaking` tests (rust-lang/cargo#14049) - test: migrate clean to snapbox (rust-lang/cargo#14096) - Allow `unexpected_builtin_cfgs` lint in `user_specific_cfgs` test (rust-lang/cargo#14153) - test: migrate search, source_replacement and standard_lib to snapbox (rust-lang/cargo#14151) - Docs: Update config summary to include missing keys. (rust-lang/cargo#14145) - test: migrate `dep_info/diagnostics/direct_minimal_versions` to snapbox (rust-lang/cargo#14143) - Docs: Remove duplicate `strip` section. (rust-lang/cargo#14146) - Docs: Fix curly quotes in config docs. (rust-lang/cargo#14144)
What does this PR try to resolve?
Part of #14039.
Migrate following to snapbox:
tests/testsuite/registry.rstests/testsuite/registry_auth.rsAdditional information
Parameterize shared logic over inline snapshots to prevent the content from being appended multiple times in
str![]when it's called multiple times within a reusable function.Big thanks to @weihanglo for helping me out!