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 binary_name to snapbox #14041

Merged
merged 2 commits into from
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions crates/cargo-test-support/src/compare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,12 @@ pub fn assert_ui() -> snapbox::Assert {
regex::Regex::new("Finished.*in (?<redacted>[0-9]+(\\.[0-9]+))s").unwrap(),
)
.unwrap();
// output from libtest
subs.insert(
"[ELAPSED]",
regex::Regex::new("; finished in (?<redacted>[0-9]+(\\.[0-9]+))s").unwrap(),
)
.unwrap();
subs.insert(
"[FILE_SIZE]",
regex::Regex::new("(?<redacted>[0-9]+(\\.[0-9]+)([a-zA-Z]i)?)B").unwrap(),
Expand Down Expand Up @@ -156,6 +162,12 @@ pub fn assert_e2e() -> snapbox::Assert {
regex::Regex::new("[FINISHED].*in (?<redacted>[0-9]+(\\.[0-9]+))s").unwrap(),
)
.unwrap();
// output from libtest
subs.insert(
"[ELAPSED]",
regex::Regex::new("; finished in (?<redacted>[0-9]+(\\.[0-9]+))s").unwrap(),
)
.unwrap();
subs.insert(
"[FILE_SIZE]",
regex::Regex::new("(?<redacted>[0-9]+(\\.[0-9]+)([a-zA-Z]i)?)B").unwrap(),
Expand Down
92 changes: 54 additions & 38 deletions tests/testsuite/binary_name.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
#![allow(deprecated)]
//! Tests for `cargo-features = ["different-binary-name"]`.
use cargo_test_support::install::{
assert_has_installed_exe, assert_has_not_installed_exe, cargo_home,
};
use cargo_test_support::install::assert_has_installed_exe;
use cargo_test_support::install::assert_has_not_installed_exe;
use cargo_test_support::install::cargo_home;
use cargo_test_support::prelude::*;
use cargo_test_support::project;
use cargo_test_support::str;

#[cargo_test]
fn gated() {
Expand All @@ -29,7 +31,17 @@ fn gated() {
p.cargo("build")
.masquerade_as_nightly_cargo(&["different-binary-name"])
.with_status(101)
.with_stderr_contains("[..]feature `different-binary-name` is required")
.with_stderr_data(str![[r#"
[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
Caused by:
feature `different-binary-name` is required
The package requires the Cargo feature called `different-binary-name`, but that feature is not stabilized in this version of Cargo ([..]).
Consider adding `cargo-features = ["different-binary-name"]` to the top of Cargo.toml (above the [package] table) to tell Cargo you are opting in to use this unstable feature.
See https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#different-binary-name for more information about the status of this feature.
"#]])
.run();
}

Expand Down Expand Up @@ -97,12 +109,11 @@ fn binary_name1() {
// Run cargo second time, to verify fingerprint.
p.cargo("build -p foo -v")
.masquerade_as_nightly_cargo(&["different-binary-name"])
.with_stderr(
"\
[FRESH] foo [..]
[FINISHED] [..]
",
)
.with_stderr_data(str![[r#"
[FRESH] foo v0.0.1 ([ROOT]/foo)
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
"#]])
.run();

// Run cargo clean.
Expand Down Expand Up @@ -180,19 +191,30 @@ fn binary_name2() {
// Check if `cargo test` works
p.cargo("test")
.masquerade_as_nightly_cargo(&["different-binary-name"])
.with_stderr(
"\
[COMPILING] foo v0.0.1 ([CWD])
[FINISHED] `test` profile [unoptimized + debuginfo] target(s) in [..]
[RUNNING] [..] (target/debug/deps/foo-[..][EXE])",
)
.with_stdout_contains("test tests::check_crabs ... ok")
.with_stderr_data(str![[r#"
[COMPILING] foo v0.0.1 ([ROOT]/foo)
[FINISHED] `test` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
[RUNNING] unittests src/main.rs (target/debug/deps/foo-[..][EXE])
"#]])
.with_stdout_data(str![[r#"
running 1 test
test tests::check_crabs ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in [ELAPSED]s
"#]])
.run();

// Check if `cargo run` is able to execute the binary
p.cargo("run")
.masquerade_as_nightly_cargo(&["different-binary-name"])
.with_stdout("Hello, crabs!")
.with_stdout_data(str![[r#"
Hello, crabs!
"#]])
.run();

p.cargo("install")
Expand All @@ -202,7 +224,10 @@ fn binary_name2() {
assert_has_installed_exe(cargo_home(), "007bar");

p.cargo("uninstall")
.with_stderr("[REMOVING] [ROOT]/home/.cargo/bin/007bar[EXE]")
.with_stderr_data(str![[r#"
[REMOVING] [ROOT]/home/.cargo/bin/007bar[EXE]
"#]])
.masquerade_as_nightly_cargo(&["different-binary-name"])
.run();

Expand Down Expand Up @@ -253,7 +278,10 @@ fn check_env_vars() {
.run();
p.cargo("run")
.masquerade_as_nightly_cargo(&["different-binary-name"])
.with_stdout("007bar")
.with_stdout_data(str![[r#"
007bar
"#]])
.run();
p.cargo("test")
.masquerade_as_nightly_cargo(&["different-binary-name"])
Expand Down Expand Up @@ -284,25 +312,13 @@ fn check_msg_format_json() {
.file("src/main.rs", "fn main() { assert!(true) }")
.build();

let output = r#"
{
"reason": "compiler-artifact",
"package_id": "path+file:///[..]/foo#0.0.1",
"manifest_path": "[CWD]/Cargo.toml",
"target": "{...}",
"profile": "{...}",
"features": [],
"filenames": "{...}",
"executable": "[ROOT]/foo/target/debug/007bar[EXE]",
"fresh": false
}
{"reason":"build-finished", "success":true}
"#;

// Run cargo build.
p.cargo("build --message-format=json")
.masquerade_as_nightly_cargo(&["different-binary-name"])
.with_json(output)
.with_stdout_data(str![[r#"
{"executable":"[ROOT]/foo/target/debug/007bar[EXE]","features":[],"filenames":"{...}","fresh":false,"manifest_path":"[ROOT]/foo/Cargo.toml","package_id":"path+[ROOTURL]/foo#0.0.1","profile":"{...}","reason":"compiler-artifact","target":"{...}"}
Copy link
Member Author

Choose a reason for hiding this comment

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

This is something worth fixing in snapbox. We need a pretty-printed JSON lines support.

Copy link
Contributor

@epage epage Jun 11, 2024

Choose a reason for hiding this comment

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

Technically, this is no longer jsonlines. It looks like cargo-test-support had a baked in policy of taking expected and using double-newlines to delimit pretty-printed json. This is an important distinction because snapbox also supports loading jsonlines from file.

One trick I can look into is we could parse a json array and then convert that to jsonlines.

Copy link
Member Author

@weihanglo weihanglo Jun 11, 2024

Choose a reason for hiding this comment

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

Yeah I am aware of the double-newlines trick in cargo-test-support. I don't consider it as a blocker for this test file. For test suites like tests/testsuite/message_format.rs we should consider postponing the migrations.

{"reason":"build-finished","success":true}
"#]].json_lines())
.run();
}