Some patches from poi 3.17#1520
Merged
Merged
Conversation
…ror messages on failure
…parts (";" delimited) and update unit test to expect the same values as Excel. Also added tests for the failing formats.
…ror messages on failure
# Conflicts: # testcases/main/HPSF/Extractor/TestHPSFPropertiesExtractor.cs # testcases/main/HSSF/EventUserModel/TestFormatTrackingHSSFListener.cs # testcases/main/HSSF/Extractor/TestExcelExtractor.cs # testcases/main/HSSF/UserModel/TestBugs.cs # testcases/main/POIFS/FileSystem/TestNPOIFSFileSystem.cs # testcases/main/SS/UserModel/TestDataFormatter.cs # testcases/ooxml/TestXMLPropertiesTextExtractor.cs # testcases/ooxml/XSSF/Streaming/TestSXSSFWorkbook.cs # testcases/ooxml/XWPF/Extractor/TestXWPFWordExtractor.cs # testcases/ooxml/XWPF/UserModel/TestXWPFSDT.cs # testcases/ooxml/XWPF/UserModel/TestXWPFSmartTag.cs
Member
|
Please list the poi bug id in the description |
Member
|
Can you help solve the conflict? |
Collaborator
Author
|
Conflict was resolved. |
Member
|
你这一大早就开始修bug啦,可以啊 |
There was a problem hiding this comment.
Pull Request Overview
This PR applies several patches from poi 3.17 aimed at modernizing the test assertions, enhancing test coverage, and refining formatting logic. Key changes include:
- Replacing ClassicAssert calls with POITestCase assertion methods across multiple test files.
- Adjusting naming (e.g. renaming bug51675 to Bug51675) and adding new tests such as for conditional ranges and large file handling.
- Improving locale handling and conditional format detection in DataFormatter, plus minor documentation updates.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| testcases/ooxml/XWPF/UserModel/TestXWPFSmartTag.cs | Updated assertion methods to POITestCase.AssertContains. |
| testcases/ooxml/XWPF/UserModel/TestXWPFSDT.cs | Replaced ClassicAssert calls with POITestCase assertions. |
| testcases/ooxml/XWPF/Extractor/TestXWPFWordExtractor.cs | Refined text extraction assertions with POITestCase.AssertStarts/EndsWith. |
| testcases/ooxml/TestXMLPropertiesTextExtractor.cs | Updated assertions to use POITestCase. |
| testcases/main/SS/UserModel/TestDataFormatter.cs | Added TestConditionalRanges and updated fraction formatting assertions. |
| testcases/main/SS/UserModel/BaseTestWorkbook.cs | Made base test class inherit from POITestCase. |
| testcases/main/POIFS/FileSystem/TestNPOIFSFileSystem.cs | Adjusted free block assertions and removed unused using directives. |
| testcases/main/HSSF/UserModel/TestBugs.cs | Renamed test method to adhere to naming conventions and updated assertions. |
| testcases/main/HSSF/Extractor/TestExcelExtractor.cs | Added extractor.Close() calls for resource cleanup. |
| testcases/main/HSSF/EventUserModel/TestFormatTrackingHSSFListener.cs | Swapped expected format strings order to match intended outputs. |
| testcases/main/HPSF/Extractor/TestHPSFPropertiesExtractor.cs | Refined assertions using AssertContains for property extraction tests. |
| ooxml/XSSF/Streaming/SXSSFWorkbook.cs | Enhanced documentation regarding compression of temporary files. |
| main/SS/UserModel/DataFormatter.cs | Improved conditional format logic and locale pattern handling. |
| main/SS/UserModel/BuiltinFormats.cs | Adjusted built-in format ordering for consistency. |
Comments suppressed due to low confidence (2)
testcases/main/HSSF/EventUserModel/TestFormatTrackingHSSFListener.cs:63
- The expected format string for index 42 appears to have been swapped with the one for index 41. Please verify that the change is intentional.
ClassicAssert.AreEqual("_(\"$\"* #,##0_);_(\"$\"* (#,##0);_(\"$\"* "-"_);_(@_)", listener.GetFormatString(42));
main/SS/UserModel/DataFormatter.cs:329
- The condition for handling conditional data formats has been altered from checking for 3+ parts to 2+ parts; please confirm that this change aligns with Excel's behavior and does not inadvertently affect the processing of formats.
if (formatStr.IndexOf(';') != -1 && (formatStr.IndexOf(';') != formatStr.LastIndexOf(';') || rangeConditionalPattern.IsMatch(formatStr)))
Member
|
我去。。。copilot review 牛逼啊~~ |
Collaborator
Author
|
出差,事不多,就搞搞代码 |
This was referenced Jul 30, 2025
This was referenced Dec 2, 2025
This was referenced Apr 2, 2026
This was referenced May 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Main changes:
Use assertContains instead of assertTrue(text.contains) for better error messages on failure
POI 61007, use CellFormat for all format strings containing multiple parts (";" delimited) and update unit test to expect the same values as Excel. Also added tests for the failing formats.
bug 61049 fix ordering of builtin formulas