Skip to content

Add missing @Test annotation to OptionalTest.testIssue372Deser() - #393

Merged
cowtowncoder merged 1 commit into
FasterXML:2.xfrom
dylanpulver:test/collect-issue-372-deser
Oct 22, 2026
Merged

cowtowncoder merged 1 commit into
FasterXML:2.xfrom
dylanpulver:test/collect-issue-372-deser

Conversation

@dylanpulver

Copy link
Copy Markdown
Contributor

The regression test for #372 is not run.

It was written on 2.18, where OptionalTest was still collected by method name through a JUnit 3 base class. 2.19 had already migrated to JUnit 5 in #344, so the merge from 2.18 carried the method across without an annotation.

ModuleTestBase does not extend TestCase and nothing in the file calls the method, so it has not run since 2.18.5 shipped the fix in October 2025.

mvn test -Dtest=OptionalTest in datatypes goes from 13 tests to 14. The method passes as written, so this adds coverage without changing behaviour. Removing the annotation again drops it back to 13.

Test-only, so CLA-exempt per CONTRIBUTING#paperwork.

The regression test for FasterXML#372 was written on `2.18`, where this class still
extended a JUnit 3 `TestCase` base and methods were collected by name. `2.19`
had already migrated to JUnit 5 in FasterXML#344, so the merge from `2.18` carried the
method over without an annotation and nothing has run it since.

`ModuleTestBase` does not extend `TestCase` and nothing in the file calls the
method, so it has been dead since 2.18.5 shipped the fix in October 2025.

`mvn test -Dtest=OptionalTest` in `datatypes` goes from 13 tests to 14. The
method passes as written, so this adds coverage without changing behaviour.

@JooHyukKim JooHyukKim 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.

LGTM

@cowtowncoder

Copy link
Copy Markdown
Member

Thank you, @dylanpulver !

@cowtowncoder
cowtowncoder merged commit 58dab9f into FasterXML:2.x Oct 22, 2026
4 checks passed
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.

3 participants