Skip to content

test: cover field-level converter precedence#906

Merged
psxjoy merged 1 commit intoapache:mainfrom
skytin1004:test/converter-edge-case-coverage
Apr 30, 2026
Merged

test: cover field-level converter precedence#906
psxjoy merged 1 commit intoapache:mainfrom
skytin1004:test/converter-edge-case-coverage

Conversation

@skytin1004
Copy link
Copy Markdown
Contributor

@skytin1004 skytin1004 commented Apr 30, 2026

Purpose of the pull request

I've added test coverage for converter precedence when both a field-level converter and a registered converter could apply to the same Java type.

The behavior I wanted to lock down is that a converter declared directly on a field with @ExcelProperty(converter = ...) should take precedence over a converter registered through registerConverter(...).

What's changed?

Added a regression test in CustomConverterTest for this case.

The test writes a CSV with two String fields. One field has a field-level converter declared with @ExcelProperty, and the other field has no field-level converter, so it should use the registered String converter.

The expected output contains both converted values: field:value,registered:value.

This verifies the registered converter is actually active for the unannotated field and the field-level converter still takes precedence for the annotated field.

This is useful because users may register a general converter as a default behavior, while still expecting field-specific converters to override it for special cases.

Tested with fesod-sheet -Dmaven.test.skip=false -Dtest=CustomConverterTest test.

Result: Tests run: 7, Failures: 0, Errors: 0, Skipped: 0.

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

@skytin1004
Copy link
Copy Markdown
Contributor Author

Just noticed that #901 also changes CustomConverterTest.java. If #901 is merged first, I will rebase this PR and move this test to the new structure.

@psxjoy psxjoy self-requested a review April 30, 2026 07:55
Copy link
Copy Markdown
Member

@psxjoy psxjoy left a comment

Choose a reason for hiding this comment

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

lgtm

@psxjoy psxjoy merged commit 7606303 into apache:main Apr 30, 2026
9 checks passed
@skytin1004 skytin1004 deleted the test/converter-edge-case-coverage branch April 30, 2026 08:33
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