Skip to content

Add unit tests for ProtobufUtil#710

Open
vasiliy-mikhailov wants to merge 1 commit into
infinispan:mainfrom
vasiliy-mikhailov:add-ProtobufUtil-tests
Open

Add unit tests for ProtobufUtil#710
vasiliy-mikhailov wants to merge 1 commit into
infinispan:mainfrom
vasiliy-mikhailov:add-ProtobufUtil-tests

Conversation

@vasiliy-mikhailov

@vasiliy-mikhailov vasiliy-mikhailov commented Jun 24, 2026

Copy link
Copy Markdown

Additive unit tests for ProtobufUtil — edge cases and current behavior pinned with explicit assertions. No existing test or production code changed (append-only).

Verified green under Java 17 (mvn -pl core test -Dtest=ProtobufUtilTest → 59 tests, 0 failures). On this class the additions raise line coverage from 42 to 65/69 lines and the PIT mutation kill-rate from 65 to 109/121 mutants.


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.

@vasiliy-mikhailov
vasiliy-mikhailov requested a review from a team as a code owner June 24, 2026 06:53

@tristantarrant tristantarrant left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the contribution !

Most of the calls are invoking thin wrapper overloads, so I don't think they are worth the tests.
Please keep the tests for the following:

  • testWriteToNullObject
  • testFromEnumClass
  • testEstimateSize
  • testToByteBuffer / testFromByteBuffer

@tristantarrant

Copy link
Copy Markdown
Member

Also, can you create an issue and format the commit message according to the commit rules ?

@vasiliy-mikhailov
vasiliy-mikhailov force-pushed the add-ProtobufUtil-tests branch 2 times, most recently from 6e9833b to c91399e Compare July 8, 2026 12:40
@vasiliy-mikhailov

Copy link
Copy Markdown
Author

Thanks @tristantarrant! Addressed both points:

  • Trimmed the PR down to just the five tests you flagged (testWriteToNullObject, testFromEnumClass, testEstimateSize, testToByteBuffer / testFromByteBuffer) and dropped the thin-wrapper-overload tests.
  • Created issue Improve unit-test coverage for ProtobufUtil #720 and reformatted the commit to start with [#720] per the commit rules.

Ready for another look whenever you have a moment.

@tristantarrant

Copy link
Copy Markdown
Member

Sorry, you need to adjust to the new Jupiter assertion method argument order (the message goes in last position now)

@vasiliy-mikhailov

Copy link
Copy Markdown
Author

Thanks @tristantarrant! Addressed the Jupiter feedback:

  • Rebased onto current main (which migrated the suite to JUnit 5 Jupiter) and reordered the assertion in testEstimateSize so the message is in last position, per the new Jupiter argument order.
  • The PR is now exactly the five tests you asked to keep: testWriteToNullObject, testFromEnumClass, testEstimateSize, testToByteBuffer, testFromByteBuffer. The diff is just those 50 lines.
  • Linked to issue Improve unit-test coverage for ProtobufUtil #720, and the commit uses the [#720] format.

Verified locally under JDK 17: ProtobufUtilTest compiles and all of its tests pass. CI is re-running for 17/21/25-ea.

🤖 Addressed by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants