Skip to content

Commit c71ae5a

Browse files
authored
fix: make release lint baseline-aware (#10678)
* fix: make release lint baseline-aware * style: format baseline-aware lint changes
1 parent ee36bf6 commit c71ae5a

7 files changed

Lines changed: 897 additions & 204 deletions

File tree

crates/homeboy-extension/src/lint/run/exit_code.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,13 @@ pub(super) fn normalize_producer_exit_code(
4242
}
4343
}
4444

45-
pub(super) fn effective_lint_exit_code(exit_code: i32, baseline_exit_override: Option<i32>) -> i32 {
45+
pub(super) fn effective_lint_exit_code(
46+
exit_code: i32,
47+
baseline_exit_override: Option<i32>,
48+
hard_error: bool,
49+
) -> i32 {
4650
match baseline_exit_override {
47-
Some(0) if exit_code >= 2 => exit_code,
51+
Some(0) if hard_error => exit_code.max(1),
4852
Some(override_code) => override_code,
4953
None => exit_code,
5054
}

crates/homeboy-extension/src/lint/run/scoping.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ pub(super) fn resolve_scoped_lint_runs(
3939
files
4040
};
4141

42+
let changed_files = component_relative_changed_files(component, changed_files);
4243
if changed_files.is_empty() {
4344
println!("No files in working tree changes");
4445
return Ok(Some(Vec::new()));
@@ -56,6 +57,7 @@ pub(super) fn resolve_scoped_lint_runs(
5657
None => git::get_files_changed_since(&component.local_path, git_ref)?,
5758
};
5859

60+
let changed_files = component_relative_changed_files(component, changed_files);
5961
if changed_files.is_empty() {
6062
println!("No files changed since {}", git_ref);
6163
return Ok(Some(Vec::new()));
@@ -67,6 +69,21 @@ pub(super) fn resolve_scoped_lint_runs(
6769
}
6870
}
6971

72+
fn component_relative_changed_files(
73+
component: &Component,
74+
changed_files: Vec<String>,
75+
) -> Vec<String> {
76+
let Some(prefix) = git::get_component_path_prefix(&component.local_path) else {
77+
return changed_files;
78+
};
79+
let prefix = format!("{}/", prefix.trim_end_matches('/'));
80+
81+
changed_files
82+
.into_iter()
83+
.filter_map(|file| file.strip_prefix(&prefix).map(str::to_string))
84+
.collect()
85+
}
86+
7087
pub(super) fn build_changed_lint_runs(
7188
component: &Component,
7289
changed_files: &[String],

crates/homeboy-extension/src/lint/run/tests/exit_code.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ fn crashed_zero_finding_producer_remains_failure() {
6666

6767
#[test]
6868
fn baseline_clean_override_honors_known_findings_but_not_infrastructure_errors() {
69-
assert_eq!(effective_lint_exit_code(1, Some(0)), 0);
70-
assert_eq!(effective_lint_exit_code(2, Some(0)), 2);
69+
assert_eq!(effective_lint_exit_code(1, Some(0), false), 0);
70+
assert_eq!(effective_lint_exit_code(2, Some(0), true), 2);
71+
assert_eq!(effective_lint_exit_code(1, Some(0), true), 1);
72+
assert_eq!(effective_lint_exit_code(0, Some(0), true), 1);
7173
}

crates/homeboy-extension/src/lint/run/tests/workflow.rs

Lines changed: 268 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ use super::super::workflow::{
22
finish_scoped_lint_run_dir, run_main_lint_workflow, run_self_check_lint_workflow,
33
};
44
use super::{component, lint_args};
5-
use homeboy_core::component::{Component, ComponentScriptsConfig};
5+
use homeboy_core::component::{Component, ComponentScriptsConfig, ScopedExtensionConfig};
66
use homeboy_core::engine::run_dir::RunDir;
7+
use std::collections::HashMap;
8+
use std::path::Path;
79

810
#[test]
911
fn test_run_self_check_lint_workflow() {
@@ -60,6 +62,271 @@ fn test_run_main_lint_workflow() {
6062
assert!(result.findings.is_none());
6163
}
6264

65+
fn routed_lint_component(home: &Path, source: &Path, script: &str) -> Component {
66+
let extension_dir = home.join(".config/homeboy/extensions/routed-lint-fixture");
67+
std::fs::create_dir_all(&extension_dir).expect("extension dir");
68+
std::fs::write(
69+
extension_dir.join("routed-lint-fixture.json"),
70+
r#"{
71+
"name":"Routed lint fixture",
72+
"version":"1.0.0",
73+
"lint":{
74+
"extension_script":"lint.sh",
75+
"changed_file_routes":[
76+
{"extensions":["php"],"step":"php"},
77+
{"extensions":["js"],"step":"js"}
78+
]
79+
}
80+
}"#,
81+
)
82+
.expect("extension manifest");
83+
std::fs::write(extension_dir.join("lint.sh"), script).expect("lint script");
84+
#[cfg(unix)]
85+
{
86+
use std::os::unix::fs::PermissionsExt;
87+
let script_path = extension_dir.join("lint.sh");
88+
let mut permissions = std::fs::metadata(&script_path)
89+
.expect("script metadata")
90+
.permissions();
91+
permissions.set_mode(0o755);
92+
std::fs::set_permissions(script_path, permissions).expect("executable script");
93+
}
94+
95+
Component {
96+
id: "fixture".to_string(),
97+
local_path: source.to_string_lossy().to_string(),
98+
extensions: Some(HashMap::from([(
99+
"routed-lint-fixture".to_string(),
100+
ScopedExtensionConfig::default(),
101+
)])),
102+
..Default::default()
103+
}
104+
}
105+
106+
fn routed_lint_args() -> super::super::types::LintRunWorkflowArgs {
107+
let mut args = lint_args();
108+
args.changed_since = Some("v1.0.0".to_string());
109+
args.precomputed_changed_files =
110+
Some(vec!["legacy.php".to_string(), "assets/app.js".to_string()]);
111+
args.json_summary = true;
112+
args
113+
}
114+
115+
fn producer_sidecar_paths(
116+
workflow: &super::super::types::LintRunWorkflowResult,
117+
) -> Vec<std::path::PathBuf> {
118+
workflow
119+
.producer_summaries
120+
.iter()
121+
.filter_map(|producer| producer.source.as_ref()?.path.as_ref())
122+
.map(std::path::PathBuf::from)
123+
.collect()
124+
}
125+
126+
#[test]
127+
fn multi_route_lint_aggregates_later_route_findings() {
128+
homeboy_core::test_support::with_isolated_home(|home| {
129+
let source = tempfile::tempdir().expect("source dir");
130+
let component = routed_lint_component(
131+
home.path(),
132+
source.path(),
133+
r#"#!/bin/sh
134+
if [ "$HOMEBOY_STEP" = "php" ]; then
135+
printf '[]' > "$HOMEBOY_LINT_FINDINGS_FILE"
136+
exit 0
137+
fi
138+
printf '[{"tool":"eslint","message":"later route finding","fingerprint":"later","file":"assets/app.js"}]' > "$HOMEBOY_LINT_FINDINGS_FILE"
139+
exit 1
140+
"#,
141+
);
142+
let run_dir = RunDir::create().expect("run dir");
143+
144+
let workflow =
145+
run_main_lint_workflow(&component, source.path(), routed_lint_args(), &run_dir)
146+
.expect("workflow result");
147+
148+
assert_eq!(workflow.status, "failed");
149+
assert_eq!(workflow.exit_code, 1);
150+
let findings = workflow.findings.as_ref().expect("findings");
151+
assert_eq!(findings.len(), 1);
152+
assert_eq!(findings[0].message, "later route finding");
153+
assert_eq!(workflow.producer_summaries[1].step.as_deref(), Some("js"));
154+
assert!(producer_sidecar_paths(&workflow)
155+
.iter()
156+
.all(|path| path.is_file()));
157+
});
158+
}
159+
160+
#[test]
161+
fn multi_route_failure_retains_later_success_evidence() {
162+
homeboy_core::test_support::with_isolated_home(|home| {
163+
let source = tempfile::tempdir().expect("source dir");
164+
let component = routed_lint_component(
165+
home.path(),
166+
source.path(),
167+
r#"#!/bin/sh
168+
if [ "$HOMEBOY_STEP" = "php" ]; then
169+
printf '[{"tool":"phpcs","message":"first route finding","fingerprint":"first","file":"legacy.php"}]' > "$HOMEBOY_LINT_FINDINGS_FILE"
170+
exit 1
171+
fi
172+
printf '[]' > "$HOMEBOY_LINT_FINDINGS_FILE"
173+
exit 0
174+
"#,
175+
);
176+
let run_dir = RunDir::create().expect("run dir");
177+
178+
let workflow =
179+
run_main_lint_workflow(&component, source.path(), routed_lint_args(), &run_dir)
180+
.expect("workflow result");
181+
182+
assert_eq!(workflow.status, "failed");
183+
assert_eq!(workflow.findings.as_ref().map(Vec::len), Some(1));
184+
let paths = producer_sidecar_paths(&workflow);
185+
assert_eq!(paths.len(), 2);
186+
assert!(paths.iter().all(|path| path.is_file()));
187+
});
188+
}
189+
190+
#[test]
191+
fn successful_multi_route_lint_cleans_child_route_evidence() {
192+
homeboy_core::test_support::with_isolated_home(|home| {
193+
let source = tempfile::tempdir().expect("source dir");
194+
let component = routed_lint_component(
195+
home.path(),
196+
source.path(),
197+
r#"#!/bin/sh
198+
printf '[]' > "$HOMEBOY_LINT_FINDINGS_FILE"
199+
exit 0
200+
"#,
201+
);
202+
let run_dir = RunDir::create().expect("run dir");
203+
204+
let workflow =
205+
run_main_lint_workflow(&component, source.path(), routed_lint_args(), &run_dir)
206+
.expect("workflow result");
207+
208+
assert_eq!(workflow.status, "passed");
209+
let paths = producer_sidecar_paths(&workflow);
210+
assert_eq!(paths.len(), 2);
211+
assert!(
212+
paths[0].is_file(),
213+
"primary run evidence remains caller-owned"
214+
);
215+
assert!(!paths[1].exists(), "successful child route must be cleaned");
216+
});
217+
}
218+
219+
#[test]
220+
fn nested_component_scope_uses_existing_component_relative_file_paths() {
221+
homeboy_core::test_support::with_isolated_home(|home| {
222+
let repo = tempfile::tempdir().expect("repo dir");
223+
let run_git = |args: &[&str]| {
224+
let output = std::process::Command::new("git")
225+
.args(args)
226+
.current_dir(repo.path())
227+
.output()
228+
.expect("git command");
229+
assert!(
230+
output.status.success(),
231+
"git {:?} failed: {}",
232+
args,
233+
String::from_utf8_lossy(&output.stderr)
234+
);
235+
};
236+
run_git(&["init", "-q"]);
237+
run_git(&["config", "user.email", "homeboy@example.com"]);
238+
run_git(&["config", "user.name", "Homeboy Test"]);
239+
let component_path = repo.path().join("packages/fixture");
240+
std::fs::create_dir_all(&component_path).expect("component dir");
241+
std::fs::write(component_path.join("initial.php"), "<?php\n").expect("initial source");
242+
run_git(&["add", "packages/fixture/initial.php"]);
243+
run_git(&["commit", "-q", "-m", "initial"]);
244+
run_git(&["tag", "fixture-v1.0.0"]);
245+
std::fs::write(component_path.join("changed.php"), "<?php echo 1;\n")
246+
.expect("changed source");
247+
std::fs::write(repo.path().join("outside.php"), "<?php echo 2;\n").expect("outside source");
248+
run_git(&["add", "packages/fixture/changed.php", "outside.php"]);
249+
run_git(&["commit", "-q", "-m", "change nested and outside files"]);
250+
251+
let component = routed_lint_component(
252+
home.path(),
253+
&component_path,
254+
r#"#!/bin/sh
255+
changed="$(cat "$HOMEBOY_LINT_CHANGED_FILES_FILE")"
256+
test -f "$HOMEBOY_LINT_GLOB" || exit 2
257+
printf '[{"tool":"fixture","message":"%s|%s","fingerprint":"scope","file":"changed.php"}]' "$HOMEBOY_LINT_GLOB" "$changed" > "$HOMEBOY_LINT_FINDINGS_FILE"
258+
exit 1
259+
"#,
260+
);
261+
let run_dir = RunDir::create().expect("run dir");
262+
let mut args = lint_args();
263+
args.changed_since = Some("fixture-v1.0.0".to_string());
264+
args.json_summary = true;
265+
266+
let workflow = run_main_lint_workflow(&component, &component_path, args, &run_dir)
267+
.expect("workflow result");
268+
269+
let expected_glob = component_path.join("changed.php");
270+
assert!(expected_glob.is_file());
271+
assert_eq!(
272+
workflow.findings.as_ref().unwrap()[0].message,
273+
format!("{}|changed.php", expected_glob.display())
274+
);
275+
let manifest = run_dir.step_file(homeboy_core::engine::run_dir::files::LINT_CHANGED_FILES);
276+
assert!(manifest.is_file());
277+
assert_eq!(std::fs::read_to_string(manifest).unwrap(), "changed.php\n");
278+
assert!(!workflow.findings.as_ref().unwrap()[0]
279+
.message
280+
.contains("packages/fixture/packages/fixture"));
281+
});
282+
}
283+
284+
#[test]
285+
fn accepted_baseline_does_not_hide_later_route_producer_error() {
286+
homeboy_core::test_support::with_isolated_home(|home| {
287+
let source = tempfile::tempdir().expect("source dir");
288+
let mut known = homeboy_core::finding::HomeboyFinding::builder("eslint", "known finding")
289+
.fingerprint("known")
290+
.build();
291+
known.location.file = Some("assets/app.js".to_string());
292+
crate::lint::baseline::save_baseline(source.path(), "fixture", &[known])
293+
.expect("save baseline");
294+
let component = routed_lint_component(
295+
home.path(),
296+
source.path(),
297+
r#"#!/bin/sh
298+
if [ "$HOMEBOY_STEP" = "php" ]; then
299+
printf '[]' > "$HOMEBOY_LINT_FINDINGS_FILE"
300+
exit 0
301+
fi
302+
printf '[{"tool":"eslint","message":"known finding","fingerprint":"known","file":"assets/app.js"}]' > "$HOMEBOY_LINT_FINDINGS_FILE"
303+
printf '[{"tool":"eslint","status":"error","finding_count":1}]' > "$HOMEBOY_LINT_PRODUCERS_FILE"
304+
exit 0
305+
"#,
306+
);
307+
let run_dir = RunDir::create().expect("run dir");
308+
309+
let workflow =
310+
run_main_lint_workflow(&component, source.path(), routed_lint_args(), &run_dir)
311+
.expect("workflow result");
312+
313+
assert_eq!(workflow.status, "failed");
314+
assert_eq!(workflow.exit_code, 1);
315+
assert_eq!(workflow.findings.as_ref().map(Vec::len), Some(1));
316+
assert!(workflow
317+
.producer_summaries
318+
.iter()
319+
.any(|producer| producer.step.as_deref() == Some("js") && producer.status == "error"));
320+
assert_eq!(
321+
workflow
322+
.baseline_comparison
323+
.as_ref()
324+
.map(|comparison| comparison.new_items.len()),
325+
Some(0)
326+
);
327+
});
328+
}
329+
63330
#[test]
64331
fn lint_config_deserializes_changed_file_routes() {
65332
let config: crate::LintConfig = serde_json::from_str(

0 commit comments

Comments
 (0)