Skip to content

Fix two windows-latest test flakes: TestNPOI1469 rounding and POIDataSamples.GetFile sharing mode - #1867

Merged
tonyqus merged 2 commits into
nissl-lab:masterfrom
swyfft-insurance:upstream-pr/windows-test-flakes
Sep 12, 2026
Merged

tonyqus merged 2 commits into
nissl-lab:masterfrom
swyfft-insurance:upstream-pr/windows-test-flakes

Conversation

@ken-swyfft

Copy link
Copy Markdown
Contributor

windows-latest on master has been red on its last two pushes (runs 33473168767 on 2026-09-01 and 34364474895 on 2026-09-09) for a single test each time, TestNPOI1469, and a fork of this repo hit a second single-test failure on the same job. Both are test-infrastructure races, not library bugs. This PR fixes both. Every other test in those runs passed on both TFMs, so with these two changes the Windows job should go green.

1. TestNPOI1469 ("Row mismatch (0)", testcases/main/HSSF/UserModel/TestCellStyle.cs)

The test formats its start time with ToString("HH:mm:ss"), which truncates, and compares against DataFormatter, which rounds the Excel OLE-date double. Whenever DateTime.UtcNow lands in the upper half of a second the two disagree by one second. Truncating the start time to whole seconds makes both paths agree. This fix has been running in the swyfft-insurance fork since July with no recurrence.

2. TestDetectAsPOIFS (IOException: SampleSS.xls ... being used by another process)

POIDataSamples.GetFile() opens sample files with FileMode.OpenOrCreate, which means FileAccess.ReadWrite plus FileShare.Read. Nuke runs the net472 and net10.0 test hosts in parallel, so while TestHSSFWorkbook.CloseDoesNotModifyWorkbook holds SampleSS.xls through that handle, the other host's ConfirmIsPOIFS reader is refused. Linux has no such sharing rule, which is why ubuntu-latest never sees it. No caller writes through this handle: the one Write() that follows it is asserted to throw on a read-only filesystem, and the WriteFileSystem() calls in TestNPOIFSFileSystem target temp files. Opening read-only with FileShare.ReadWrite removes the race without changing any test's behaviour. Seen on swyfft-insurance/npoi run 34372163455 (attempt 1); the re-run of the same commit passed.

Verified locally on this branch: NPOI.TestCases builds on net10.0, and TestNPOI1469, TestOfficeXMLException, and CloseDoesNotModifyWorkbook pass. Both changes are confined to testcases/main.

(PR opened by Claude, an AI assistant, on Ken's behalf.)

ken-swyfft and others added 2 commits September 9, 2026 09:28
DateTime.UtcNow has sub-second precision. ToString("HH:mm:ss") truncates
(e.g., 16:56:17.987 -> "16:56:17"), but DataFormatter rounds the Excel
OLE date double, producing "16:56:18". Truncate to whole seconds before
the test starts so both paths agree.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
(cherry picked from commit a479c13)
…GetFile

GetFile opened test-data files with FileMode.OpenOrCreate, which means
FileAccess.ReadWrite and FileShare.Read. While TestHSSFWorkbook held
SampleSS.xls that way, any concurrent reader in the other test host was
refused with "The process cannot access the file ... because it is being
used by another process", and the net472 and net10.0 hosts run in
parallel under Nuke. TestDetectAsPOIFS failed this way on windows-latest
for #5 (first attempt); the re-run passed.

No caller writes through this handle: the only Write() that follows one
is asserted to throw on a read-only filesystem, and the WriteFileSystem
calls in TestNPOIFSFileSystem target temp files. Opening read-only with
FileShare.ReadWrite removes the race without changing any test's
behaviour.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit 66dd377)
@ken-swyfft

Copy link
Copy Markdown
Contributor Author

One more data point for the GetFile() half of this PR: #1868 (a csproj-metadata-only change on top of current master) just failed windows-latest on a third test with the same signature, Bug59058 in NPOI.OOXML.TestCases (net10.0):

System.IO.IOException : The process cannot access the file '...\testcases\test-data\document\bug59058.docx' because it is being used by another process

That test also goes through POIDataSamples.GetDocumentInstance().GetFile(), so the fix here covers it as well. That makes three distinct tests (TestDetectAsPOIFS, Bug59058, plus whatever GetFile caller loses the race next time) hitting the same OpenOrCreate handle.

(Comment by Claude, an AI assistant, on Ken's behalf.)

@tonyqus tonyqus added the cicd label Sep 12, 2026
@tonyqus tonyqus added this to the NPOI 2.8.1 milestone Sep 12, 2026
@tonyqus

tonyqus commented Sep 12, 2026

Copy link
Copy Markdown
Member

LGTM

@tonyqus
tonyqus merged commit b3d3b06 into nissl-lab:master Sep 12, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants