Skip to content

Test: expand unit coverage for scalar converters in ConverterTest - #999

Merged
delei merged 3 commits into
apache:mainfrom
leehaut:test/expand-converter-unit-tests
Aug 16, 2026
Merged

Test: expand unit coverage for scalar converters in ConverterTest#999
delei merged 3 commits into
apache:mainfrom
leehaut:test/expand-converter-unit-tests

Conversation

@leehaut

@leehaut leehaut commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Purpose of the pull request

Improve unit test coverage for individual scalar converter implementations.
ConverterTest previously only exercised FloatNumberConverter write conversion;
this change turns it into a focused @Tag(unit) suite covering the common
Boolean / Number / String converters.

What's changed?

  • Rewrite ConverterTest as pure unit tests (no Excel I/O / round-trip).
  • Add parameterized checks for supportJavaTypeKey() / supportExcelTypeKey() across 31 converters.
  • Cover read/write conversion for:
    • Boolean, Integer, Long, Short, Byte, Float, Double
    • BigDecimal, BigInteger
    • String (STRING / BOOLEAN / NUMBER / ERROR)
  • Keep factory method naming separate from the test method (supportKeysProvider) to avoid JUnit @MethodSource name clashes.

No functional changes are included in this PR.

Copilot AI 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.

Pull request overview

This pull request refactors ConverterTest into a focused unit-test suite to improve coverage of scalar converter implementations in fesod-sheet, validating supported Java/Excel type keys and basic read/write conversions without Excel I/O.

Changes:

  • Converted ConverterTest from a round-trip/integration-style test to a @Tag(unit) suite.
  • Added a parameterized test to assert supportJavaTypeKey() / supportExcelTypeKey() across a broad set of scalar converters.
  • Added targeted read/write conversion assertions for Boolean/Number/String converter variants (including BigDecimal/BigInteger).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@delei delei 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

@delei
delei merged commit e120395 into apache:main Aug 16, 2026
9 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.

4 participants