From 6be242a534e3ba06cbb6d86a1c82b17b94e4e81e Mon Sep 17 00:00:00 2001 From: Frank O'Hara Date: Tue, 11 Aug 2026 13:40:31 -0600 Subject: [PATCH] test: kill eight surviving mutants in formatter, parser and blocker analysis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by mutation-testing lash 0.3.1 with Flawd 0.8.0 (400 mutants sampled from 3528, per-test targeting, every survivor re-confirmed against the full suite). These eight survivors are all cases where a conditional's two branches produce the same test outcome, so inverting it changed nothing that any test observed. Three sit in code 0.3.1 just changed: - formatter: `normalize_whitespace` was only ever exercised through the helper it gates, never through the flag itself, so inverting the flag passed. The trailing-blank-line trim and the "don't re-emit a description that is already in the source" guard were likewise unasserted. - parser: a blank line between a checkbox and its annotation block. The parsed values did not pin this — annotations are recovered downstream either way — but `annotation_line_count` silently drops to 0, and that count is what the write paths use to find where a task's annotations end. A wrong count there is how `lash add` splices a task into a previous task's block, the data-loss class fixed in 0.3.1 (#49). - blocker analysis: `find_blocker_chains` was only tested on a chain one hop deep, where the walk exits on the "current task is not itself blocked" check before its own progress guard is ever what stops it. A four-node chain distinguishes them. The report's per-blocker status word and its chains section were also unpinned. Each test was verified to fail against its mutation and pass without it. No production code changed. One further survivor is left deliberately untested: in the parser's blank-line branch, `} else if !annotation_lines.is_empty() { break; }` is followed by an unconditional `break`, so both paths break identically and no test can observe the difference. It is an equivalent mutant, not a gap; the redundant branch could be removed but that is a change to production code and not this commit's business. --- .../src/dependency/blocker_analyzer.rs | 125 ++++++++++++++++++ crates/lash-core/src/formatter/mod.rs | 80 +++++++++++ crates/lash-core/src/parser/mod.rs | 25 ++++ 3 files changed, 230 insertions(+) diff --git a/crates/lash-core/src/dependency/blocker_analyzer.rs b/crates/lash-core/src/dependency/blocker_analyzer.rs index 68d7cb1..aff1544 100644 --- a/crates/lash-core/src/dependency/blocker_analyzer.rs +++ b/crates/lash-core/src/dependency/blocker_analyzer.rs @@ -1044,4 +1044,129 @@ mod tests { let blockers = analyzer.find_blockers("test#task1"); assert_eq!(blockers.len(), 0); } + + /// Walking a chain past its second link. + /// + /// `test_find_blocker_chains` stops at task1 → task2 → task3, which is one + /// hop: the walk pushes task3 and then exits because task3 is not itself + /// blocked. The loop's own "did I find anything this pass?" guard is never + /// what ends it, so breaking on that guard's inverse produced the same + /// two-element chain and nothing failed. A fourth node forces a second + /// hop, which is the first case that distinguishes them. + #[test] + fn test_blocker_chain_follows_more_than_one_hop() { + let mut graph = DependencyGraph::new(); + for (id, title) in [ + ("test#task1", "Task 1"), + ("test#task2", "Task 2"), + ("test#task3", "Task 3"), + ("test#task4", "Task 4"), + ] { + graph.add_node( + id.to_string(), + create_node(title, TaskStatus::Open, "test", 0), + ); + } + for (from, to) in [ + ("test#task1", "test#task2"), + ("test#task2", "test#task3"), + ("test#task3", "test#task4"), + ] { + graph.add_edge( + from.to_string(), + to.to_string(), + EdgeData::new(DependencyKind::ExplicitId, None), + ); + } + + let computer = StatusComputer::new(&graph); + let statuses = computer.compute_all(); + let analyzer = BlockerAnalyzer::new(&graph, &statuses); + + let chains = analyzer.find_blocker_chains("test#task1"); + assert_eq!(chains.len(), 1); + + let chain = &chains[0]; + assert_eq!( + chain.len(), + 3, + "the walk should follow task2 → task3 → task4, not stop after the first hop" + ); + assert_eq!(chain.direct_blocker().unwrap().task_id, "test#task2"); + assert_eq!(chain.root_blocker().unwrap().task_id, "test#task4"); + } + + fn blocker_with_status(task_id: &str, status: ComputedStatus) -> BlockerInfo { + BlockerInfo::new( + task_id.to_string(), + format!("Title of {task_id}"), + "test".to_string(), + 0, + DependencyKind::ExplicitId, + status, + ) + } + + /// `BlockerReport::format` is rendered for the user but only its blocker + /// counts were asserted, so the per-blocker status word and the chains + /// section were unpinned. + #[test] + fn test_report_format_labels_each_blocker_status() { + let blocked = blocker_with_status("test#blocked", ComputedStatus::Blocked(Vec::new())); + let incomplete = blocker_with_status("test#incomplete", ComputedStatus::Incomplete); + let report = BlockerReport::new( + "test#task1".to_string(), + "Task 1".to_string(), + vec![blocked.clone(), incomplete.clone()], + vec![BlockerChain::new( + "test#task1".to_string(), + vec![blocked, incomplete.clone()], + )], + vec![incomplete], + Vec::new(), + ); + + let output = report.format(); + + assert!( + output.contains("[blocked]"), + "a blocker that is itself blocked should be labelled, got:\n{output}" + ); + assert!( + output.contains("[incomplete]"), + "a root blocker should be labelled incomplete, got:\n{output}" + ); + assert!( + output.contains("Blocker Chains:"), + "a report carrying chains should show the chains section, got:\n{output}" + ); + } + + /// The other side of the chains-section condition: with no chains the + /// heading must not appear above an empty list. + #[test] + fn test_report_format_omits_chains_section_when_there_are_none() { + let report = BlockerReport::new( + "test#task1".to_string(), + "Task 1".to_string(), + vec![blocker_with_status( + "test#incomplete", + ComputedStatus::Incomplete, + )], + Vec::new(), + Vec::new(), + Vec::new(), + ); + + let output = report.format(); + + assert!( + !output.contains("Blocker Chains:"), + "no chains means no chains section, got:\n{output}" + ); + assert!( + output.contains("All Blockers:"), + "the blocker list itself should still render, got:\n{output}" + ); + } } diff --git a/crates/lash-core/src/formatter/mod.rs b/crates/lash-core/src/formatter/mod.rs index 6fdb79f..7ee5f05 100644 --- a/crates/lash-core/src/formatter/mod.rs +++ b/crates/lash-core/src/formatter/mod.rs @@ -1061,4 +1061,84 @@ mod tests { // Multiple blank lines collapsed assert!(!result.contains("\n\n\n\n")); } + + /// Format `source` with `normalize_whitespace` set either way. + /// + /// The parse is what production does — `format_file_in_place` parses the + /// same source it hands the formatter — so these tests exercise the real + /// path rather than a hand-built `TaskFile`. + fn format_with_normalize(source: &str, normalize: bool) -> String { + let config = LashConfig::default(); + let file = + crate::parser::parse_file_from_string(source, &config).expect("fixture should parse"); + let options = FormatOptions { + normalize_whitespace: normalize, + ..FormatOptions::default() + }; + Formatter::new(config, options) + .format_file(source, &file) + .expect("formatting should succeed") + } + + /// `test_whitespace_normalization` above calls `normalize_whitespace` + /// directly, so nothing pinned the flag that decides whether it runs at + /// all: inverting it left every test passing. This asserts both settings + /// produce their own result. + #[test] + fn normalize_whitespace_option_decides_whether_blank_runs_collapse() { + let source = "# Demo\n\n@id: demo\n\n## Notes\n\nalpha\n\n\n\n\nbeta\n\n## Tasks\n\n- [ ] First task\n"; + + let collapsed = format_with_normalize(source, true); + assert!( + !collapsed.contains("\n\n\n\n"), + "normalize_whitespace = true should collapse the blank run, got: {collapsed:?}" + ); + + let preserved = format_with_normalize(source, false); + assert!( + preserved.contains("\n\n\n\n"), + "normalize_whitespace = false should leave the blank run alone, got: {preserved:?}" + ); + } + + /// The trailing-newline branch only runs when the output actually ends in + /// a blank line, which `normalize_whitespace` otherwise hides by stripping + /// trailing blanks first. With it off, the generated tasks section leaves a + /// blank line at EOF and this is the only thing that trims it. + #[test] + fn format_ends_file_with_exactly_one_newline() { + let source = "# Demo\n\n@id: demo\n\n## Tasks\n\n- [ ] First task\n"; + + let formatted = format_with_normalize(source, false); + + assert!( + formatted.ends_with('\n'), + "formatted file should end with a newline, got: {formatted:?}" + ); + assert!( + !formatted.ends_with("\n\n"), + "formatted file should not end with a blank line, got: {formatted:?}" + ); + } + + /// A file with no `## Tasks` heading gets one synthesized. The description + /// must not be synthesized alongside it when the source already had a + /// `## Description` section — the walk copied that through already, so + /// emitting it again duplicates it. + #[test] + fn synthesized_tasks_section_does_not_duplicate_an_existing_description() { + let source = "# Demo\n\n@id: demo\n\n## Description\n\nProse that belongs to the file.\n"; + + let formatted = format_with_normalize(source, true); + + assert_eq!( + formatted.matches("Prose that belongs to the file.").count(), + 1, + "description should appear exactly once, got: {formatted:?}" + ); + assert!( + formatted.contains("## Tasks"), + "a missing tasks section should still be synthesized, got: {formatted:?}" + ); + } } diff --git a/crates/lash-core/src/parser/mod.rs b/crates/lash-core/src/parser/mod.rs index b28adfc..f38fb91 100644 --- a/crates/lash-core/src/parser/mod.rs +++ b/crates/lash-core/src/parser/mod.rs @@ -1903,6 +1903,31 @@ More text assert_eq!(task.metadata.docs.len(), 1); } + /// A blank line is allowed between a checkbox and its annotation block. + /// + /// The parsed *values* are not enough to pin this: when the block is not + /// consumed here, the annotations are still recovered downstream, so + /// `owner` and `labels` look correct either way. What breaks is + /// `annotation_line_count`, which drops to 0 because these lines were + /// never attributed to the task. That count is what the write paths use to + /// find where a task's annotations end, so a wrong count is how `lash add` + /// splices a new task into the middle of a previous task's block — the + /// data-loss class fixed in 0.3.1. Assert the count, not just the values. + #[test] + fn test_blank_line_before_annotations_still_counts_them() { + let content = "# Test File\n\n## Tasks\n\n- [ ] Task one\n\n@owner: bob\n@labels: x, y\n"; + let config = LashConfig::default(); + + let file = parse_file_from_string(content, &config).expect("fixture should parse"); + let task = file.tasks.tasks().first().expect("one task expected"); + + assert_eq!( + task.annotation_line_count, 2, + "both annotation lines belong to the task even across the blank line" + ); + assert_eq!(task.metadata.owner, Some("bob".to_string())); + } + // ==================== Description Section Integration Tests ==================== #[test]