You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fesod.apache.org by "pjfanning (via GitHub)" <gi...@apache.org> on 2026/10/16 14:12:17 UTC
[PR] test: check DataFormatter against text computed by Excel [fesod]
pjfanning opened a new pull request, #1231:
URL: https://github.com/apache/fesod/pull/1231
Adds `DataFormatterAgainstExcelTest`, which checks Fesod's `DataFormatter` (`org.apache.fesod.sheet.metadata.format.DataFormatter`) against text produced by Excel itself. It is a port of POI's `TestDataFormatterAgainstExcel` (apache/poi#1398).
The goal is regression coverage. If Fesod moves to POI's `DataFormatter`, or changes its own, this test shows exactly which outputs change.
## How it works
Several sample workbooks in POI's test data contain `TEXT(value, format)` formulas. Excel cached their results when it saved the file, so each cached string is Excel's own rendering of that value and format. The workbooks are copied into `fesod-sheet/src/test/resources/dataformat/excel/` (Apache-licensed, from apache/poi `test-data/spreadsheet`):
`NumberFormatTests.xlsx`, `GeneralFormatTests.xlsx`, `FormatConditionTests.xlsx`, `FormatChoiceTests.xlsx`, `ElapsedFormatTests.xlsx`, `DateFormatTests.xlsx`, `DateFormatNumberTests.xlsx` (1904 date system, which the test honours), `DateTimeToNumberTestCases.xls` and `59106.xlsx`.
The test formats each value the way Fesod does when reading: `new DataFormatter(date1904, Locale.US, false).format(BigDecimal, index, format)`. The `false` is Fesod's default `useScientificFormat`. Each format gets its own index, because `DateUtils.isADateFormat` caches its result by format index.
Cases that don't match Excel are listed, grouped by cause:
- `UNSUPPORTED_FORMATS`: formats that never match.
- `KNOWN_FAILURES`: specific format and value pairs.
Each listed case is asserted to *still differ*. A change that fixes one fails the test until its entry is removed; a change that breaks a working case fails as usual.
It also covers the Chinese date/time, `[DBNum]` and quoted-literal cases from POI's `TestDataFormatter` (apache/poi#1400, which came out of apache/poi#1397). Those expectations come from POI's tests, not from Excel-cached values.
## Results: 569 Excel cases, 151 match
| Workbook | Match |
|---|---|
| NumberFormatTests.xlsx | 36 / 325 |
| GeneralFormatTests.xlsx | 39 / 67 |
| FormatConditionTests.xlsx | 12 / 33 |
| FormatChoiceTests.xlsx | 10 / 24 |
| ElapsedFormatTests.xlsx | 10 / 50 |
| DateFormatTests.xlsx | 23 / 45 |
| DateFormatNumberTests.xlsx | 6 / 10 |
| DateTimeToNumberTestCases.xls | 14 / 14 |
| 59106.xlsx | 1 / 1 |
For comparison, POI trunk matched 297 of these 569 before apache/poi#1398, and 429 with it.
### What works
- Plain number formats: `0.00`, `#,##0.00`, `#,###`, `0,000`, `$###.##`, `0.00%`, `#,##0.00%`, `$000,000,000`
- Number formats with currency symbols and quoted text: `\a $#.00" profit"`, `"before "¥#.00`
- Common date and time formats: `dd-mm-yy`, `dddd-mmmm-yyyy`, `dd mmm yyyy`, `dd\ mmm\ yyyy`, `yyyy/mm`, `h:m:s`, `hh:mm`, `H:MM AM/PM`, `h:m:s.00`
- `yyyy-mm-dd hh:mm:ss` with 37.638 seconds shows `:38`, as Excel does. **POI truncates this to `:37`**, so switching to POI's class would regress this case. POI lists it as a known failure.
- All of `DateTimeToNumberTestCases.xls`
- `[DBNum1-3]` formats, and quoted 年/月/日 literals, e.g. `yyyy"年"m"月"d"日"`
### What doesn't (from the lists in the test)
**Number formats**
- A comma between digit placeholders is taken as a custom grouping size, not grouping by thousands. `0,00` shows 1234567 as `1,23,45,67`; Excel shows `1,234,567`.
- **Trailing-comma scaling is wrong.** The issue #2319 change also divides by ten for each character between the decimal point and the comma. `0,` shows 1234567 as `124` (Excel `1235`), `#,,` as `0` (Excel `1`), and `0.#,` as `123.5` (Excel `1234.6`). POI handles the #2319 format, `0\.0,`, correctly since apache/poi#1396.
- `?` digit placeholders outside fractions
- Fractions with text or spacing between the digits
- `%%`
- A trailing decimal point with no digits after it (`#,##.#`)
- `General` with very large or very small numbers, which Excel shows in scientific notation. Fesod only does this when `useScientificFormat` is set.
- Single-section conditional formats such as `[<10]#" Wow"`: the text is dropped.
**Date and time formats**
- A single `y` is a 2-digit year (`d-m-y`).
- `A/P` / `a/p` should show a single letter.
- Escaped letters and quoted text: `d "days" h` and `d \d\a\y\s h` are not taken as date formats.
- Fractions of a second are dropped (`hh:mm:ss.000` shows `.000`), or the seconds are rounded up while the fraction is still shown (`:38.64` instead of `:37.64`).
- `[s].000`; elapsed units not at the start or mixed with text; negative elapsed times.
**Chinese and quoted-literal cases from POI (19 of 36 fail)**
- `mm` after quoted or Chinese text is read as the month, so `h"时"mm"分"` shows **`14时03分`** instead of `14时24分`.
- With `上午/下午`, the marker follows the locale: Excel always shows 上午/下午, but Fesod shows `PM` under en_US.
- Formats with `上午/下午` before the time, e.g. `上午/下午h:mm`, are not detected as dates.
- `0.00"m/秒"` is taken as a date format, so 1.5 shows `0.001/秒`. This comes from the loose 年月日时分秒 check in `ExcelDateFormatDetector`.
- `hh"h"mm`: the `h` inside the quotes is used as a pattern letter.
## Test
`./mvnw -pl fesod-sheet test -Dmaven.test.skip=false -Dtest=DataFormatterAgainstExcelTest` gives 605 tests, all passing. `spotless:check` is clean.
No production code is changed.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] test: check DataFormatter against text computed by Excel [fesod]
Posted by "bengbengbalabalabeng (via GitHub)" <gi...@apache.org>.
bengbengbalabalabeng merged PR #1231:
URL: https://github.com/apache/fesod/pull/1231
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] test: check DataFormatter against text computed by Excel [fesod]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1231:
URL: https://github.com/apache/fesod/pull/1231#issuecomment-6217415630
## Test Results
1 750 tests 1 750 ✅ 1m 31s ⏱️
129 suites 0 💤
129 files 0 ❌
Results for commit b1cb6232.
[test-results]:data:application/gzip;base64,H4sIAEcx0moC/2WMSw7CIBQAr9KwdgG0IHgZAw+avNgWw2dlvLuImqIuZyaZG5lx8YmcBsb1YSCpYO7QlWgyhq0KzSrXlls9CvrBcyoAf+6C1+p2MRtcvoSPMcS3iWXrrk/6mb7U/mzcLRv3RwjrirkCsQys5CMXSo7UMu6c9w60EpOYqPTMaW2NB0XuD0iz5goMAQAA
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org