Skip to content

test: CRLF parity tests assert exit codes, so they pass on both sides of the fix #32

Open

Description

@sugat009

Summary

tests/crlf_parity.rs is byte-identical on main and at #28's head (diff reports no
difference). All three of its tests assert only .failure():

test assertion
crlf_commit_is_rejected_via_hash tests/crlf_parity.rs:85 .failure();
crlf_commit_is_rejected_via_from_hash tests/crlf_parity.rs:94 .failure();
crlf_message_file_is_rejected tests/crlf_parity.rs:104 .failure();

#28 changes newline handling completely, and all three still pass, because exit 1 is exit 1.
For the commit message feat: abcd\r\nbody line via --hash:

binary exit output
main 1 Commit validation: failed! (no error detail at all)
#28 head 1 Description cannot contain line breaks.
commitlint 2.0.0 1 Description cannot contain line breaks.

main rejects it because its single regex requires a literal \n\n before a body; it rejects
the plain-LF feat: abcd\nbody line the same way, so the \r is not what it is reacting to.
#28 normalizes the input first and then reports a specific parity-matching error. Two unrelated
mechanisms, one assertion, no signal. AGENTS.md already says "Assert the exact message, not just
the exit code."

Note also that the input these tests use is not the divergence they were written for. Both tools
reject feat: abcd\r\nbody line. main's actual divergence was a CRLF file with a body, which
main rejects and upstream accepts, and nothing in the file used that input.

The module doc is now false

tests/crlf_parity.rs:13-17 says:

cocox exits 1 on that same input today, for the separate reason that it implements only
upstream's simple pattern, so do not read this as the two agreeing on argument handling.

At #28's head, cocox $'feat: add\r\nbody line' exits 0 and so does commitlint 2.0.0. They agree.
The rest of that module doc (lines 3 to 11, the --hash claim) still measures correctly.

The new --file CRLF tests do not discriminate

#28 adds file_with_crlf_normalized (tests/cli.rs:1338), file_with_lone_cr_normalized
(tests/cli.rs:1345) and crlf_header_length_counted_correctly (tests/cli.rs:1354). Delete the
normalize_newlines call from read_file in src/command.rs:13 and the whole suite still passes:
188 tests, 0 failures. Delete the two calls in src/git_helpers.rs:24 and :74 instead and two
tests fail, crlf_commit_is_rejected_via_hash and crlf_commit_is_rejected_via_from_hash, both
with "Unexpected success", code 0. crlf_message_file_is_rejected passes through that mutation too.

The reason is that --file sets strip_comments (src/command.rs:115), so remove_comments
runs, and it is lines().filter(...).join("\n") (src/utils.rs:39-47). Rust's lines() already
strips a trailing \r from each line, so \r\n is folded before any validator sees it, with or
without normalize_newlines. Only a lone \r with no following \n survives, and no test uses
one where it changes the verdict. Here is an input that does:

printf 'feat: add new feature\rbody line' > e.txt
cocox --file e.txt

head and upstream both exit 1 with "Description cannot contain line breaks."; with the read_file
normalization removed, cocox exits 0. That gap is currently untested.

Done looks like

  1. The three tests in tests/crlf_parity.rs assert the exact stderr, not .failure(), so that a
    change in why cocox rejects an input fails the test.
  2. tests/crlf_parity.rs:13-17 is rewritten to what the two tools now do, or deleted.
  3. One --file test covers a lone \r case whose verdict depends on read_file normalization,
    for example feat: add new feature\rbody line. Deleting normalize_newlines from
    src/command.rs:13 must fail at least one test.
  4. Verify 3 by actually deleting that call in a scratch copy and watching the suite go red.

Most of what this touches lands with #28; the .failure() assertions and the stale module doc
are already on main.

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

    Type

    No type

    Projects

    • Status
      In progress

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions