Skip to content

Commit 40cdc33

Browse files
committed
fix(filter-quality): preserve error signal in go test/vet/golangci filters
Every retry these filters caused had the same root: RTK kept the cosmetic summary but dropped the one actionable line (the error/failure message), then advertised a verbose tee-log -- so agents retried and pulled back more bytes than the unfiltered output. - go test: drop the tee for 'go test'. The [full output: ...go_test.log] pointer handed out the raw 'go test -json' dump (3-8x verbose); agents cat'd it on failure. Build errors and per-test failures are already inline. - go vet: keep every finding line, not just '.go:' lines. Location-less compiler/cgo failures (fatal error: pcap.h: No such file or directory) were dropped, then reported as 'No issues found' on a hard failure. Stop truncating the message tail mid-line. - golangci-lint: emit standard 'file:line:col: message (linter)' findings. The violation message (Text) was parsed and discarded, leaving only counts. - golangci-lint: passthrough verbatim when the user supplies --out-format / --output.* instead of force-parsing JSON and emitting a parse-error string that reads as a tool failure. Savings stay high vs raw (golangci 78-84%); go vet passes tiny failures through by design (compressing 323 B is negative value).
1 parent 0a630fe commit 40cdc33

2 files changed

Lines changed: 221 additions & 103 deletions

File tree

src/cmds/go/go_cmd.rs

Lines changed: 116 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,16 @@ pub fn run_test(args: &[String], verbose: u8) -> Result<i32> {
7171
filter_go_test_json
7272
};
7373

74+
// No tee: the raw stream is `go test -json` (3–8× more verbose than native
75+
// output). A `[full output: …go_test.log]` pointer just advertises that
76+
// firehose — agents cat it and pull back more bytes than the unfiltered run.
77+
// The filter surfaces build errors and per-test failure detail inline instead.
7478
runner::run_filtered(
7579
cmd,
7680
"go test",
7781
&args.join(" "),
7882
filter,
79-
crate::core::runner::RunOptions::stdout_only().tee("go_test"),
83+
crate::core::runner::RunOptions::stdout_only(),
8084
)
8185
}
8286

@@ -279,6 +283,27 @@ fn run_go_tool_golangci_lint(args: &[OsString], verbose: u8) -> Result<i32> {
279283
&*stdout
280284
};
281285

286+
let exit_code = exit_code_from_output(&output, "go tool golangci-lint");
287+
// golangci-lint: exit 0 = clean, exit 1 = lint issues found (not an error),
288+
// exit 2+ = config/build error, None = killed by signal (OOM, SIGKILL)
289+
let mapped_exit = if exit_code == 1 { 0 } else { exit_code };
290+
291+
// User chose their own output format — emit it verbatim rather than parsing
292+
// it as JSON (which would fail and surface a parse-error string).
293+
if has_format {
294+
print!("{}", stdout);
295+
if !stderr.trim().is_empty() {
296+
eprint!("{}", stderr);
297+
}
298+
timer.track(
299+
"go tool golangci-lint",
300+
"rtk go tool golangci-lint (passthrough)",
301+
&raw,
302+
&raw,
303+
);
304+
return Ok(mapped_exit);
305+
}
306+
282307
let filtered = golangci_cmd::filter_golangci_json(json_output, version);
283308
println!("{}", filtered);
284309

@@ -293,10 +318,7 @@ fn run_go_tool_golangci_lint(args: &[OsString], verbose: u8) -> Result<i32> {
293318
&filtered,
294319
);
295320

296-
let exit_code = exit_code_from_output(&output, "go tool golangci-lint");
297-
// golangci-lint: exit 0 = clean, exit 1 = lint issues found (not an error),
298-
// exit 2+ = config/build error, None = killed by signal (OOM, SIGKILL)
299-
Ok(if exit_code == 1 { 0 } else { exit_code })
321+
Ok(mapped_exit)
300322
}
301323

302324
/// Parse go test -json output (NDJSON format)
@@ -697,35 +719,39 @@ fn is_go_build_error_line(line: &str) -> bool {
697719
|| lower.contains("function main is undeclared in the main package")
698720
}
699721

700-
/// Filter go vet output - show issues
722+
/// Filter go vet output - show issues.
723+
///
724+
/// vet only prints when something is wrong, so every non-`#` line is signal —
725+
/// including location-less compiler/cgo failures (`fatal error: …`) that have
726+
/// no `.go:`. Filtering on `.go:` dropped those and reported "No issues found"
727+
/// on a hard failure; truncation cut the message tail an agent retries to read.
701728
fn filter_go_vet(output: &str) -> String {
702-
let mut issues: Vec<String> = Vec::new();
703-
704-
for line in output.lines() {
705-
let trimmed = line.trim();
706-
707-
// Collect issue lines (vet reports issues with file:line:col format)
708-
if !trimmed.is_empty() && !trimmed.starts_with('#') && trimmed.contains(".go:") {
709-
issues.push(trimmed.to_string());
710-
}
711-
}
729+
let issues: Vec<&str> = output
730+
.lines()
731+
.map(str::trim)
732+
.filter(|line| !line.is_empty() && !line.starts_with('#'))
733+
.collect();
712734

713735
if issues.is_empty() {
714736
return "Go vet: No issues found".to_string();
715737
}
716738

717-
let mut result = String::new();
718-
result.push_str(&format!("Go vet: {} issues\n", issues.len()));
739+
let mut result = format!("Go vet: {} issues\n", issues.len());
719740

720741
const MAX_GO_VET_ISSUES: usize = CAP_ERRORS;
721742
for (i, issue) in issues.iter().take(MAX_GO_VET_ISSUES).enumerate() {
722-
result.push_str(&format!("{}. {}\n", i + 1, truncate(issue, 120)));
743+
result.push_str(&format!("{}. {}\n", i + 1, issue));
723744
}
724745

725746
if issues.len() > MAX_GO_VET_ISSUES {
726-
result.push_str(&format!("\n… +{} more issues\n", issues.len() - MAX_GO_VET_ISSUES));
747+
result.push_str(&format!(
748+
"\n… +{} more issues\n",
749+
issues.len() - MAX_GO_VET_ISSUES
750+
));
727751
let all_issues = issues.join("\n");
728-
if let Some(hint) = crate::core::tee::force_tee_tail_hint(&all_issues, "go-vet", MAX_GO_VET_ISSUES + 1) {
752+
if let Some(hint) =
753+
crate::core::tee::force_tee_tail_hint(&all_issues, "go-vet", MAX_GO_VET_ISSUES + 1)
754+
{
729755
result.push_str(&format!(" {}\n", hint));
730756
}
731757
}
@@ -901,6 +927,36 @@ mod tests {
901927
);
902928
}
903929

930+
#[test]
931+
fn test_filter_go_test_surfaces_cgo_build_error_inline() {
932+
// A cgo build failure (missing C header) must show the compiler error
933+
// line inline — this is the one actionable fact. With no tee pointer,
934+
// the agent has no firehose to fall back to, so the signal must be here.
935+
let output = r##"{"Action":"start","Package":"example.com/sniff"}
936+
{"ImportPath":"example.com/sniff","Action":"build-output","Output":"# example.com/sniff\n"}
937+
{"ImportPath":"example.com/sniff","Action":"build-output","Output":"./capture.go:7:11: fatal error: pcap.h: No such file or directory\n"}
938+
{"ImportPath":"example.com/sniff","Action":"build-fail"}
939+
{"Package":"example.com/sniff","Action":"fail","FailedBuild":"example.com/sniff"}"##;
940+
941+
let result = filter_go_test_json(output);
942+
assert!(
943+
result.contains("[build failed]"),
944+
"Expected build-failed marker, got: {}",
945+
result
946+
);
947+
assert!(
948+
result.contains("pcap.h: No such file or directory"),
949+
"Compiler error line must survive inline, got: {}",
950+
result
951+
);
952+
// The "# package" header is noise and should be dropped.
953+
assert!(
954+
!result.contains("# example.com/sniff"),
955+
"Package header should be stripped, got: {}",
956+
result
957+
);
958+
}
959+
904960
#[test]
905961
fn test_filter_go_build_success() {
906962
let output = "";
@@ -1069,6 +1125,45 @@ utils.go:15:5: unreachable code"#;
10691125
assert!(result.contains("unreachable code"));
10701126
}
10711127

1128+
#[test]
1129+
fn test_filter_go_vet_preserves_location_less_cgo_error() {
1130+
// cgo/compiler failures have no `.go:` anchor — the old `.go:`-only
1131+
// filter dropped them and reported "No issues found" on a hard failure.
1132+
let output = r#"# example.com/sniff
1133+
fatal error: pcap.h: No such file or directory
1134+
compilation terminated."#;
1135+
1136+
let result = filter_go_vet(output);
1137+
assert!(
1138+
!result.contains("No issues found"),
1139+
"Must not claim success on a cgo failure, got: {}",
1140+
result
1141+
);
1142+
assert!(
1143+
result.contains("fatal error: pcap.h: No such file or directory"),
1144+
"Compiler error line must survive, got: {}",
1145+
result
1146+
);
1147+
assert!(
1148+
!result.contains("# example.com/sniff"),
1149+
"Package header should be stripped, got: {}",
1150+
result
1151+
);
1152+
}
1153+
1154+
#[test]
1155+
fn test_filter_go_vet_does_not_truncate_long_message() {
1156+
// The actionable detail of a `could not import` line lives at its tail;
1157+
// a mid-message cut is exactly what an agent retries to recover.
1158+
let long = "./capture.go:9:2: could not import github.com/google/gopacket/pcap (-: # github.com/google/gopacket/pcap: fatal error: pcap.h: No such file or directory)";
1159+
let result = filter_go_vet(long);
1160+
assert!(
1161+
result.contains("pcap.h: No such file or directory"),
1162+
"Message tail must not be truncated, got: {}",
1163+
result
1164+
);
1165+
}
1166+
10721167
#[test]
10731168
fn test_compact_package_name() {
10741169
assert_eq!(compact_package_name("github.com/user/repo/pkg"), "pkg");

0 commit comments

Comments
 (0)