-
Notifications
You must be signed in to change notification settings - Fork 466
test(core): improve assertion usage for better clarity and order #685
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@alaahong PTAL, Thanks. |
f4cc0d1 to
40213ff
Compare
delei
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
psxjoy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves test code quality by standardizing assertion parameter order across the test suite to follow JUnit conventions. The changes ensure that assertion failure messages will be clearer by displaying "Expected: X, but was: Y" format consistently.
Key changes:
- Corrected assertion parameter order from
assertEquals(actual, expected)toassertEquals(expected, actual)throughout the test suite - Modernized instance checks by replacing
assertTrue(x instanceof Type)withassertInstanceOf(Type.class, x) - Applied consistent formatting to multi-line assertions
Reviewed Changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| TemplateDataListener.java | Fixed assertion parameter order for size and string value checks |
| StyleDataListener.java | Corrected assertion order for list size and string comparisons |
| SortDataListener.java | Fixed list size assertion parameter order |
| SimpleDataTest.java | Corrected assertions and migrated to assertInstanceOf |
| SimpleDataSheetNameListener.java | Fixed assertion order for size and name checks |
| SimpleDataListener.java | Corrected multiple assertions including header map and sheet data checks |
| RepetitionDataListener.java | Fixed assertion parameter order for list and string validations |
| ParameterDataListener.java | Corrected assertion order for size, name, and sheet number checks |
| UnCamelDataListener.java | Fixed assertions for header map and camel case data validations |
| MultipleSheetsListener.java | Corrected title assertion parameter order |
| LargeDataListener.java | Fixed count assertion parameter order in conditional blocks |
| NoHeadDataListener.java | Corrected assertions for size and string data |
| ListHeadDataListener.java | Fixed list size assertion parameter order |
| ComplexDataListener.java | Corrected assertions for size and string field checks |
| FillAnnotationDataTest.java | Migrated instanceof checks to assertInstanceOf |
| FillDataTest.java | Fixed list size assertions with Long type expectations |
| ExceptionDataListener.java | Corrected assertion order for size, name, and sheet metadata |
| EncryptDataListener.java | Fixed assertion parameter order for encrypted data validations |
| ReadAllConverterDataListener.java | Corrected extensive converter assertions for numeric, string, and date types |
| ConverterDataListener.java | Fixed assertion order for various data type conversions |
| CharsetDataTest.java | Corrected list size assertion in anonymous listener |
| CellDataDataListener.java | Fixed assertions for cell data and formula validations |
| BomDataTest.java | Corrected list size assertions in BOM data tests |
| AnnotationIndexAndNameDataListener.java | Fixed assertion order for indexed data checks |
| AnnotationDataListener.java | Corrected numeric assertion parameter order |
| ExcelAnalyserOldBiffTest.java | Migrated instanceof checks to assertInstanceOf with message parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Changes
Corrected assertion parameter order
Changed from assertEquals(actual, expected) to assertEquals(expected, actual)
Follows JUnit conventions where expected value comes first
Results in clearer failure messages: "Expected: X, but was: Y"