From 37fcaa672d8115cfa9dcf88f63b0f56031fe06ed Mon Sep 17 00:00:00 2001 From: Yijie Xu <48432972+yeahjack@users.noreply.github.com> Date: Mon, 5 Oct 2026 07:10:20 -0700 Subject: [PATCH 1/2] fix(pipe): preserve non-JSON Go test reports --- src/cmds/go/go_cmd.rs | 24 ++++++++++++ src/cmds/system/pipe_cmd.rs | 32 +++++++++++++++- tests/go_test_pipe_test.rs | 73 +++++++++++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 tests/go_test_pipe_test.rs diff --git a/src/cmds/go/go_cmd.rs b/src/cmds/go/go_cmd.rs index c21b094a15..bfab4ab713 100644 --- a/src/cmds/go/go_cmd.rs +++ b/src/cmds/go/go_cmd.rs @@ -291,6 +291,30 @@ fn run_go_tool_golangci_lint(args: &[OsString], verbose: u8) -> Result { 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::(line) else { + return false; + }; + let identity = match event.action.as_str() { + "start" | "run" | "pause" | "cont" | "pass" | "bench" | "fail" | "output" | "skip" => { + 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 = HashMap::new(); diff --git a/src/cmds/system/pipe_cmd.rs b/src/cmds/system/pipe_cmd.rs index b133673ac5..d455d65d4b 100644 --- a/src/cmds/system/pipe_cmd.rs +++ b/src/cmds/system/pipe_cmd.rs @@ -41,7 +41,11 @@ pub fn resolve_filter(name: &str) -> Option 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 { @@ -355,6 +359,32 @@ 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}"#, + ] { + 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()); diff --git a/tests/go_test_pipe_test.rs b/tests/go_test_pipe_test.rs new file mode 100644 index 0000000000..8b0f1b771f --- /dev/null +++ b/tests/go_test_pipe_test.rs @@ -0,0 +1,73 @@ +//! `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::(); + 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}"); +} From fb79b81d570ec3249cc17a66b5a2af0a7baee94f Mon Sep 17 00:00:00 2001 From: Yijie Xu <48432972+yeahjack@users.noreply.github.com> Date: Mon, 5 Oct 2026 08:50:51 -0700 Subject: [PATCH 2/2] fix(pipe): reject Go output events without a payload --- src/cmds/go/go_cmd.rs | 5 +++++ src/cmds/system/pipe_cmd.rs | 2 ++ tests/go_test_pipe_test.rs | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/src/cmds/go/go_cmd.rs b/src/cmds/go/go_cmd.rs index bfab4ab713..55e537157e 100644 --- a/src/cmds/go/go_cmd.rs +++ b/src/cmds/go/go_cmd.rs @@ -300,6 +300,11 @@ pub(crate) fn is_go_test_json(input: &str) -> bool { let Ok(event) = serde_json::from_str::(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" => { event.package diff --git a/src/cmds/system/pipe_cmd.rs b/src/cmds/system/pipe_cmd.rs index d455d65d4b..c085b0bd5a 100644 --- a/src/cmds/system/pipe_cmd.rs +++ b/src/cmds/system/pipe_cmd.rs @@ -368,6 +368,8 @@ mod tests { 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); } diff --git a/tests/go_test_pipe_test.rs b/tests/go_test_pipe_test.rs index 8b0f1b771f..ec75f72e65 100644 --- a/tests/go_test_pipe_test.rs +++ b/tests/go_test_pipe_test.rs @@ -71,3 +71,40 @@ fn valid_json_events_still_use_the_existing_filter() { assert!(shown.contains("x_test.go:12"), "{shown}"); assert!(!shown.contains("No tests found"), "{shown}"); } + +#[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); +}