Skip to content
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
15 changes: 8 additions & 7 deletions crates/prek/src/cli/run/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -718,17 +718,18 @@ impl<'a> HookRunSession<'a> {
)
.await?;

let needs_diff = group_results
.iter()
.any(|result| result.file_changes == hooks::FileChanges::Unknown);
let known_modified_files = group_results
.iter()
.any(|result| result.file_changes == hooks::FileChanges::Modified);
let needs_diff = !known_modified_files
&& group_results
.iter()
.any(|result| result.file_changes == hooks::FileChanges::Unknown);
let diff_detected_modifications = diff_tracker.changed_after_group(needs_diff).await?;
if known_modified_files && !needs_diff {
// Native hooks report modifications directly, so no Git snapshot
// was taken. Force one before a later external hook needs a
// before/after comparison.
if known_modified_files {
// The group is already known to have modified files, so a Git
// comparison cannot change its result. A later external hook
// will capture the current worktree before it runs.
diff_tracker.invalidate();
}
let group_modified_files = known_modified_files || diff_detected_modifications;
Expand Down
120 changes: 120 additions & 0 deletions crates/prek/tests/skipped_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,126 @@ fn read_only_languages_do_not_run_diff_detection() -> Result<()> {
Ok(())
}

#[test]
fn same_group_known_modification_skips_diff_detection() -> Result<()> {
let context = TestContext::new();
context.init_project();

let cwd = context.work_dir();
context.write_pre_commit_config(indoc::indoc! {r#"
repos:
- repo: builtin
hooks:
- id: end-of-file-fixer
priority: 0
- repo: local
hooks:
- id: noop
name: noop
language: system
entry: python3 -c "pass"
pass_filenames: false
priority: 0
"#});

cwd.child("file.txt").write_str("missing newline")?;
context.git_add(".");

let output = context.run().env("RUST_LOG", "prek::git=trace").output()?;

assert!(
!output.status.success(),
"the builtin should report its modification"
);
assert_eq!(context.read("file.txt"), "missing newline\n");

let stdout = String::from_utf8_lossy(&output.stdout);
assert!(
stdout
.lines()
.any(|line| line.contains("noop") && line.contains("Passed"))
);

let stderr = String::from_utf8_lossy(&output.stderr);
assert_eq!(
stderr.matches("has_worktree_diff").count(),
0,
"A known modification should make the same-group quiet diff unnecessary.\n\
Trace output:\n{stderr}"
);
assert_eq!(
stderr.matches("diff_worktree").count(),
0,
"A known modification should make the same-group full diff unnecessary.\n\
Trace output:\n{stderr}"
);

Ok(())
}

#[test]
fn same_group_known_modification_rebaselines_later_external_hook() -> Result<()> {
let context = TestContext::new();
context.init_project();

let cwd = context.work_dir();
context.write_pre_commit_config(indoc::indoc! {r#"
repos:
- repo: builtin
hooks:
- id: end-of-file-fixer
priority: 0
- repo: local
hooks:
- id: same-group-noop
name: same-group-noop
language: system
entry: python3 -c "pass"
pass_filenames: false
priority: 0
- id: later-noop
name: later-noop
language: system
entry: python3 -c "pass"
pass_filenames: false
priority: 1
"#});

cwd.child("file.txt").write_str("missing newline")?;
context.git_add(".");

let output = context.run().env("RUST_LOG", "prek::git=trace").output()?;

assert!(
!output.status.success(),
"the builtin should report its modification"
);
assert_eq!(context.read("file.txt"), "missing newline\n");

let stdout = String::from_utf8_lossy(&output.stdout);
assert!(
stdout
.lines()
.any(|line| line.contains("later-noop") && line.contains("Passed"))
);

let stderr = String::from_utf8_lossy(&output.stderr);
assert_eq!(
stderr.matches("has_worktree_diff").count(),
0,
"The known modification should invalidate the clean baseline.\n\
Trace output:\n{stderr}"
);
assert_eq!(
stderr.matches("diff_worktree").count(),
2,
"The later external hook should capture and compare the modified worktree.\n\
Trace output:\n{stderr}"
);

Ok(())
}

#[test]
fn modifying_builtin_invalidates_baseline_for_later_external_hook() -> Result<()> {
let context = TestContext::new();
Expand Down
Loading