Conversation
poi-ooxml/src/main/java/org/apache/poi/xssf/binary/XSSFBSheetHandler.java
Outdated
Show resolved
Hide resolved
poi-ooxml/src/main/java/org/apache/poi/xssf/binary/XSSFBSheetHandler.java
Outdated
Show resolved
Hide resolved
poi-ooxml/src/main/java/org/apache/poi/xssf/binary/XSSFBSheetHandler.java
Outdated
Show resolved
Hide resolved
poi-ooxml/src/main/java/org/apache/poi/xssf/binary/XSSFBSheetHandler.java
Show resolved
Hide resolved
|
The CI run had a test failure. I'm travelling and bandwidth is too low to read the full log. |
|
I broke this test Test test66682() FAILED org.opentest4j.AssertionFailedError: expected: As I am now returning the actual error rather than just the generic ERROR. Options are
I can't decide which is the better option? Either is easy to do. |
|
I prefer to not break any tests. You can add a method or param that flips the behaviour but users should opt into the breaking change. This is from real world pain with changes leading to complaints from users who rely on the behaviour as is and don't expect it to change. |
|
OK I updated the code so the error behaviour works as it did before for the old string based API. Anyone who needs the actual error codes can use the new API. |
poi-ooxml/src/test/java/org/apache/poi/xssf/eventusermodel/TestXSSFBReader.java
Show resolved
Hide resolved
poi-ooxml/src/test/java/org/apache/poi/xssf/eventusermodel/TestXSSFBReader.java
Outdated
Show resolved
Hide resolved
| XSSFBSheetHandler.XSSFBSheetContentsHandler handler = mockSheetContentsHandler(); | ||
| testXSSFBSheetContentsHandler("testVarious.xlsb", handler); | ||
|
|
||
| InOrder ordered = inOrder(handler); |
There was a problem hiding this comment.
Why do we need mockito? Can't this be tested with the APIs as is?
There was a problem hiding this comment.
Because I need to test that certain callbacks get called on the XSSFBSheetContentsHandler. So I needed to either create a mock handler by hand (which is what the existing tests did) or use a third party mocking framework like mockito (that I think was already been used in other places in the code base).
This PR introduces a new API for reading data from xlsb files with their native types as opposed to the current methodology which returns everything as a formatted string.
This is done via the introduction of a new callback handler XSSFBSheetContentsHandler which has typed callbacks.
This change is backwards compatible with the existing API which is still present and works as it did before.
This PR also adds support for reading the error codes from the xlsb file and returning the actual error.