Skip to content

Harden the round-trip divergence-pinning test pattern (fail() + line-shift-fragile hunk headers) #64

Open

Description

@morisil

The dump round-trip tests that pin a captured divergence (rather than asserting a clean fixpoint) use this escape-hatch pattern:

try {
    reparsed.renderMarkdown() sameAs markdown
    error("expected the round-trip to diverge")
} catch (e: AssertionError) {
    e.message sameAs """<pinned diff>"""
}

Two robustness problems with this pattern, both raised by automated reviews on #63.

Problem A — error() instead of asserting the throw

error() throws IllegalStateException, which is not a subtype of AssertionError, so it is not caught by the catch (e: AssertionError) block. If a pinned divergence is ever fixed (the round-trip becomes a fixpoint), sameAs passes, error() propagates uncaught, and the test surfaces as a test error ("expected the round-trip to diverge") rather than a clean assertion failure.

Preferred fix — assertFailsWith (already on the classpath), which returns the caught exception and, if the block doesn't throw, fails cleanly with "expected an exception … but completed successfully":

val divergence = assertFailsWith<AssertionError> {
    reparsed.renderMarkdown() sameAs markdown
}
divergence.message sameAs """<pinned diff>"""

W3cValidatorRefsTest was migrated to this form in #63; the rest below still use error().

Problem B — the pinned diff encodes line-shift-fragile hunk headers

The e.message sameAs "..." pins the exact unified-diff output, including hunk headers like @@ -18,12 +18,16 @@. Those offsets are line numbers, so any unrelated pipeline change that shifts a line in the rendered output — a blank-line whitespace fix, an added frontmatter field, etc. — changes the hunk header and fails the test even though the actual divergence is unchanged. The test should fail only when the divergence changes, not when the surrounding output moves.

Fix options: normalize the pinned message (e.g. blank out the @@ … @@ line-number offsets before comparing), or assert only the -/+ divergence lines rather than the whole unified diff.

Scope — keep consistent (codebase-wide)

All divergence-pinning round-trip tests share the pattern:

Deferred from

PR #63 (RefMode) reviews — house style, not introduced there, kept out of that PR to avoid inconsistent one-test changes.

🤖 Generated with Claude Code

Activity

  1. added a commit that references this issue on Jun 30, 2026
  2. changed the title [-]Round-trip divergence-pinning tests use error() instead of fail() (surfaces as error, not failure)[/-] [+]Harden the round-trip divergence-pinning test pattern (fail() + line-shift-fragile hunk headers)[/+] on Jul 1, 2026
  3. added 4 commits that reference this issue on Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions