Skip to content

OPENNLP-1859: Add tests for BilouCodec encode/decode and outcome compatibility - #1135

Merged
mawiesne merged 3 commits into
apache:mainfrom
vasiliy-mikhailov:add-BilouCodec-tests
Jul 2, 2026
Merged

mawiesne merged 3 commits into
apache:mainfrom
vasiliy-mikhailov:add-BilouCodec-tests

Conversation

@vasiliy-mikhailov

Copy link
Copy Markdown
Contributor

Adds 37 JUnit 5 tests to BilouCodecTest covering edge cases in BilouCodec: dangling/standalone LAST and CONTINUE during decode, null-type span encoding (UNIT, START+LAST, and START+CONTINUE*n+LAST paths), span boundary computation, and areOutcomesCompatible validation for missing START/LAST, default-prefixed outcomes, and multi-type sequences.

metric before after
mutation score 85% 91%
test methods added n/a 37

The additions are append-only (no existing test is modified) and pass against the current code (BilouCodecTest: 81 tests green, 0 checkstyle violations under JDK 21).


How this was produced

This PR was generated with an AI-assisted pipeline built around mutation testing (PIT). The pipeline mutates the target class (flipping conditions and changing boundary/edge cases) and runs the existing tests against each mutant. Where a mutant survives (the existing tests do not catch that edge case), it writes a focused test for that case and reruns PIT to confirm the new test actually kills that specific mutant. So every added test is verified to catch a concrete edge case the suite missed before, rather than being speculative or redundant. The change is additive only (no production code modified), and the module builds green under its CI JDK.

@mawiesne mawiesne added java Pull requests that update Java code tests Pull requests that add or update test code labels Jun 30, 2026
@mawiesne
mawiesne requested review from atarora and rzo1 June 30, 2026 05:02
@mawiesne

Copy link
Copy Markdown
Contributor

@vasiliy-mikhailov Thx for the PR, team will review it shortly.

@rzo1 rzo1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Some minor nits, than good to go:

  1. Duplicate test. testCompatibilityContinueWithoutStartOrLast uses the identical input {B_CONTINUE, B_LAST, B_UNIT} as testCompatibilityContinueWithoutStart - same array, same assertion. It's pure redundancy and, despite its name/Javadoc, does not exercise the "cont with neither matching start nor last" branch (line 178's &&) it claims to, because B_LAST populates the last set. To actually hit that branch, use something like {B_CONTINUE, A_UNIT} (a cont prefix with no matching start or last, but unit non-empty so it gets past the early return false).

  2. Line-number references in Javadoc ("line 178", "lines 99-105", "line 161/173") will rot as the source shifts, and a couple are already imprecise, e.g. testCompatibilityContinueWithoutStart actually short-circuits to false in the last-validation loop (line 184), not the cont loop it cites. Harmless to behavior; consider dropping the literal line numbers in favor of naming the branch.

@mawiesne mawiesne changed the title Add edge-case tests for BilouCodec encode/decode and outcome compatibility OPENNLP-1859: Add tests for BilouCodec encode/decode and outcome compatibility Jun 30, 2026
@rzo1
rzo1 force-pushed the add-BilouCodec-tests branch from 4043d53 to 6f49e1c Compare June 30, 2026 06:11
@vasiliy-mikhailov

Copy link
Copy Markdown
Contributor Author

Small proactive follow-up: I rewrote a few of the test comments to describe the behaviour under test instead of referencing specific production line numbers, which would go stale if BilouCodec shifts. No test logic changed (still 81 green). Happy to adjust anything when the team reviews.

…CONTINUE, A_UNIT}) per review: it duplicated WithoutStart and short-circuited in the last-validation loop instead of exercising the cont-with-no-matching-start-or-last branch; 81 tests green
@vasiliy-mikhailov

Copy link
Copy Markdown
Contributor Author

Thanks @rzo1, both good catches.

On the duplicate: testCompatibilityContinueWithoutStartOrLast now uses {B_CONTINUE, A_UNIT} instead of the same {B_CONTINUE, B_LAST, B_UNIT} as testCompatibilityContinueWithoutStart, so it actually reaches the cont-with-no-matching-start-or-last branch you pointed to (rather than short-circuiting in the last-validation loop); the assertion stays incompatible. It is now a genuinely distinct case.

On the line-number comments: already handled in an earlier follow-up. I dropped the literal line 178 / lines 99-105 references from the Javadoc and reworded them to name the branch/behaviour under test, since they would rot as BilouCodec shifts (and, as you noted, testCompatibilityContinueWithoutStart short-circuits in the last-validation loop, not the cont loop I had cited).

Full test class stays green after both changes (81 tests). Thanks again for the review.

@mawiesne
mawiesne merged commit 47fd462 into apache:main Jul 2, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

java Pull requests that update Java code tests Pull requests that add or update test code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants