Skip to content
Open
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
29 changes: 29 additions & 0 deletions src/cmds/go/go_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,35 @@ fn run_go_tool_golangci_lint(args: &[OsString], verbose: u8) -> Result<i32> {
Ok(if exit_code == 1 { 0 } else { exit_code })
}

/// Recognize a complete event stream before filtering piped input. The runner
/// controls Go's output format, but `pipe` may receive plain text, unrelated JSON,
/// or merged stderr. Preserve all of those rather than silently discarding lines.
pub(crate) fn is_go_test_json(input: &str) -> bool {
let mut saw_event = false;
for line in input.lines().map(str::trim).filter(|line| !line.is_empty()) {
let Ok(event) = serde_json::from_str::<GoTestEvent>(line) else {
return false;
};
// Output actions require their payload; an Action/Package-shaped log
// object alone is not evidence that this is Go's event format.
if matches!(event.action.as_str(), "output" | "build-output") && event.output.is_none() {
return false;
}
let identity = match event.action.as_str() {
"start" | "run" | "pause" | "cont" | "pass" | "bench" | "fail" | "output" | "skip" => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocks merge (1): this accept set is stricter than what filter_go_test_json already summarizes. Real streams that develop summarizes and this head passes raw: Go ≥1.25 t.Attr ("Action":"attr", 45,295 bytes raw instead of Go test: 51 passed in 27 packages), Go ≥1.26 -artifacts ("Action":"artifacts"), go tool test2json without -p (no Package on any event), a UTF-8 BOM before line 1, a 2>&1 stream with two go: downloading lines (44,503 bytes raw instead of Go test: 51 passed in 26 packages), and a stream cut mid-object. In the other direction, a go build -json stream of only build-output/build-fail events is accepted and becomes Go test: No tests found, dropping the compile error. Rows and transcripts are in the review body.

event.package
}
"build-output" | "build-fail" => event.import_path,
_ => return false,
};
if identity.is_none_or(|name| name.is_empty()) {
return false;
}
saw_event = true;
}
saw_event
}

/// Parse go test -json output (NDJSON format)
pub(crate) fn filter_go_test_json(output: &str) -> String {
let mut packages: HashMap<String, PackageResult> = HashMap::new();
Expand Down
34 changes: 33 additions & 1 deletion src/cmds/system/pipe_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ pub fn resolve_filter(name: &str) -> Option<fn(&str) -> String> {
}

fn go_test_wrapper(input: &str) -> String {
crate::cmds::go::go_cmd::filter_go_test_json(input)
if crate::cmds::go::go_cmd::is_go_test_json(input) {
crate::cmds::go::go_cmd::filter_go_test_json(input)
} else {
input.to_string()
}
}

fn git_status_wrapper(input: &str) -> String {
Expand Down Expand Up @@ -365,6 +369,34 @@ mod tests {
assert!(resolve_filter("pytest").is_some());
}

#[test]
fn go_test_wrapper_preserves_non_event_input() {
for input in [
"",
" \n\t\n",
"ok \texample.com/proj\t0.001s\nFAIL\texample.com/other\t0.002s\n",
r#"{"Action":"pass"}"#,
r#"{"Action":"unknown","Package":"example.com/proj"}"#,
r#"{"Action":"output","Package":"example.com/proj","Output":42}"#,
r#"{"Action":"output","Package":"example.com/proj","message":"setup failed"}"#,
r#"{"Action":"build-output","ImportPath":"example.com/proj","message":"compile failed"}"#,
] {
assert_eq!(go_test_wrapper(input), input);
}
}

#[test]
fn go_test_wrapper_keeps_build_event_filtering() {
let input = concat!(
"{\"Action\":\"build-output\",\"ImportPath\":\"example.com/proj\",\"Output\":\"main.go:4: undefined: missing\\n\"}\n",
"{\"Action\":\"build-fail\",\"ImportPath\":\"example.com/proj\"}\n",
"{\"Action\":\"fail\",\"Package\":\"example.com/proj\",\"FailedBuild\":\"example.com/proj\"}\n",
);
let shown = go_test_wrapper(input);
assert!(shown.contains("main.go:4: undefined: missing"), "{shown}");
assert_ne!(shown, input);
}

#[test]
fn test_resolve_filter_go_test() {
assert!(resolve_filter("go-test").is_some());
Expand Down
110 changes: 110 additions & 0 deletions tests/go_test_pipe_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
//! `pipe` cannot recover an upstream exit status, but it must retain the report.
use std::io::Write;
use std::process::Stdio;

mod common;

fn pipe(input: &str) -> String {
let mut child = common::rtk_command()
.args(["pipe", "-f", "go-test"])
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.spawn()
.expect("spawn rtk pipe");
child
.stdin
.take()
.expect("piped stdin")
.write_all(input.as_bytes())
.expect("write input");
let out = child.wait_with_output().expect("read output");
assert_eq!(out.status.code(), Some(0));
assert!(out.stderr.is_empty(), "unexpected stderr: {:?}", out.stderr);
String::from_utf8(out.stdout).expect("UTF-8 output")
}

#[test]
fn plain_failure_after_many_successes_is_preserved_in_full() {
let mut input = (0..25)
.map(|i| format!("ok \texample.com/proj/pkg{i}\t0.001s\n"))
.collect::<String>();
input.push_str("--- FAIL: TestX (0.00s)\n x_test.go:12: expected 1, got 2\nFAIL\texample.com/proj/zz\t0.01s\nFAIL\n");
assert_eq!(pipe(&input), input);
}

#[test]
fn plain_success_empty_and_non_event_json_are_not_misclassified() {
for input in [
"ok \texample.com/proj\t0.003s\n? \texample.com/tools\t[no test files]\n",
"",
" \n\t\n",
"{\"message\":\"build failed\"}\n",
"{\"Action\":\"deploy\",\"Package\":\"example.com/proj\"}\n",
"{\"Action\":\"pass\"}\n",
"{\"Action\":\"output\",\"Package\":\"example.com/proj\",\"Output\":42}\n",
] {
assert_eq!(pipe(input), input);
}
}

#[test]
fn mixed_json_and_text_keeps_every_line() {
let events = "{\"Action\":\"pass\",\"Package\":\"example.com/proj\",\"Test\":\"TestOK\"}\n";
for input in [
format!("go: downloading example.com/dep v1.0.0\n{events}"),
format!("{events}setup failed: missing dependency\n"),
format!("{events}{{\"error\":\"not a Go test event\"}}\n"),
format!("{events}{{malformed json\n"),
] {
assert_eq!(pipe(&input), input);
}
}

#[test]
fn valid_json_events_still_use_the_existing_filter() {
let pass = "{\"Action\":\"run\",\"Package\":\"example.com/proj\",\"Test\":\"TestOK\"}\n{\"Action\":\"pass\",\"Package\":\"example.com/proj\",\"Test\":\"TestOK\"}\n{\"Action\":\"pass\",\"Package\":\"example.com/proj\"}\n";
assert_eq!(pipe(pass), "Go test: 1 passed in 1 packages");
let fail = "{\"Action\":\"output\",\"Package\":\"example.com/proj\",\"Test\":\"TestX\",\"Output\":\" x_test.go:12: expected 1, got 2\\n\"}\n{\"Action\":\"fail\",\"Package\":\"example.com/proj\",\"Test\":\"TestX\"}\n{\"Action\":\"fail\",\"Package\":\"example.com/proj\"}\n";
let shown = pipe(fail);
assert!(shown.contains("TestX"), "{shown}");
assert!(shown.contains("x_test.go:12"), "{shown}");
assert!(!shown.contains("No tests found"), "{shown}");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Blocks merge (2): removing "output" from the allowlist at go_cmd.rs:326 leaves this test green, because the input then comes back raw and the raw input also contains TestX and x_test.go:12 and not No tests found. The same is true for removing "start", "skip", "pause", "cont" or "bench": the whole suite passes. A real captured go test -json ./... stream (with a no-test-files package, a t.Parallel pair and a benchmark that calls b.Log) asserted against its exact summary line closes all six.

}

#[test]
fn raw_fallback_preserves_line_endings_and_unterminated_text() {
for input in [
" \t\r\nFAIL\texample.com/proj\t0.01s\r\n ",
"--- FAIL: TestUnicode\n expected 日本語, got 中文",
"{\"message\":\"failure without newline\"}",
] {
assert_eq!(pipe(input), input);
}
}

#[test]
fn output_shaped_json_without_output_is_not_a_test_report() {
for input in [
r#"{"Action":"output","Package":"example.com/proj","message":"setup failed"}"#,
r#"{"Action":"build-output","ImportPath":"example.com/proj","message":"compile failed"}"#,
] {
assert_eq!(pipe(input), input);
}
}

#[test]
fn padded_json_and_build_failures_keep_existing_summaries() {
let input =
" \r\n \t{\"Action\":\"pass\",\"Package\":\"example.com/proj\",\"Test\":\"TestOK\"} \r\n\t";
assert_eq!(pipe(input), "Go test: 1 passed in 1 packages");
let build = concat!(
"{\"Action\":\"build-output\",\"ImportPath\":\"example.com/proj\",\"Output\":\"main.go:4: undefined: missing\\n\"}\n",
"{\"Action\":\"build-fail\",\"ImportPath\":\"example.com/proj\"}\n",
"{\"Action\":\"fail\",\"Package\":\"example.com/proj\",\"FailedBuild\":\"example.com/proj\"}\n",
);
let shown = pipe(build);
assert!(shown.contains("main.go:4: undefined: missing"), "{shown}");
assert!(shown.contains("[build failed]"), "{shown}");
assert_ne!(shown, build);
}
Loading