diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 829e2aa..ba619d7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -175,7 +175,15 @@ jobs: export CGO_ENABLED=1 go vet -tags nohtj2k ./... - # Tests deliberately deferred — running `go test -race` on Windows with - # cgo is brittle (msys2 Go vs Windows file-system semantics) and the - # full unit suite already passes on macOS. v0.2.x will add Windows tests - # once the cgo edges are smoothed and OpenJPH is packaged. + - name: go test (Windows, unit only, htj2k disabled, no -race) + run: | + export PATH=/mingw64/bin:$PATH + export PKG_CONFIG_PATH=/mingw64/lib/pkgconfig + export CGO_ENABLED=1 + # Unit suite only — no `-tags integration`, so fixture-gated tests + # skip (Windows CI downloads no fixtures). No `-race`: race+cgo on + # msys2 Go is brittle, and the Windows job's value is platform- + # specific behavior (path separators, temp dirs, file-system + byte- + # order semantics) — race detection is already covered on macOS. + # htj2k stays disabled (OpenJPH is not packaged for msys2). + go test -tags nohtj2k -timeout 30m ./... diff --git a/docs/format-debt-survey-2026-06-13.md b/docs/format-debt-survey-2026-06-13.md index f1771c9..2cf011e 100644 --- a/docs/format-debt-survey-2026-06-13.md +++ b/docs/format-debt-survey-2026-06-13.md @@ -53,7 +53,7 @@ additionally supports dzi/szi/dicom as one-shot targets. | D1 | ~~**Integration suite never runs in CI**~~ **DONE** (PR #4, merge eea2da4) — added a `go test (integration)` step (`-tags integration ./tests/integration/...`) to the macOS job; tests skip gracefully on fixtures absent from the 3 CI pulls. CI-verified green (`ok … 10.5s`). | `.github/workflows/ci.yml` | [confirmed firsthand] | S–M | High | | D2 | ~~**No DICOM CI fixture**~~ **DONE** (PR #5, merge c51cd7b) — wsi-fixtures **v5** adds `dicom.tar`: 3DHISTECH-JP2K/HTJ2K (CC0) + scan_621_grundium_dicom (CC-BY-4.0, attribution). CI pulls it + 16 instance SHAs; the DICOM unit + integration tests now RUN in CI (integration 10.5s→33.8s). | `.github/fixtures.sha256`, `ci.yml` | [confirmed firsthand] | M (cross-repo) | High | | D3 | **No JP2K-SVS / OME-TIFF / Leica-SCN / generic-TIFF CI fixtures** → those paths skip in CI. **PARTIAL** (merge bf5c81f): wsi-fixtures **v7** added an OME-TIFF (`CMU-1-Small-Region.ome.tiff` → OME-TIFF transform CI coverage) + the `590_crop` ImageScope crops (JP2K-SVS + LZW/uncompressed TIFF). **Multi-level SVS: DONE** — `239551.svs` (owner's own scan, multi-level Aperio SVS, 3 levels + thumbnail/label/overview) landed as **wsi-fixtures v8** (`svs.tar`, CC-BY-4.0; repo PR #5) and wired into CI (`ci.yml` v7→v8 + cache key, `fixtures.sha256` sha `019946db…`); the 5 previously skip-if-absent multi-level tests (`convert --to svs` thumbnail/overview placement + `rebuildSVS` thumbnail/label/overview remove\|replace) now RUN in CI. **Still open: Leica-SCN.** Note: 239551 happens to splice on thumbnail replace, so CI coverage of the `rebuildSVS` *path* specifically would additionally want a CMU-1-style multi-level slide whose L0 data interleaves the thumbnail offsets. | fixtures.sha256 | [confirmed firsthand] | M | Med | -| D4 | **Windows CI job runs no tests** (build+vet only); HTJ2K untested on Windows (`-tags nohtj2k`). | `ci.yml` | [confirmed] | M | Med | +| D4 | ~~**Windows CI job runs no tests** (build+vet only)~~ **DONE** (PR #16) — added `go test -tags nohtj2k -timeout 30m ./...` to the Windows job: unit-only (no `-tags integration` → fixture-gated tests skip, Windows pulls no fixtures), no `-race` (race+cgo on msys2 brittle; the Windows job's value is path/FS/byte-order coverage, not race detection — that's on macOS). Full unit suite runs green on Windows incl. cgo codecs (jpeg/jpegxl/png/webp/jp2k). HTJ2K still `-tags nohtj2k` (OpenJPH unpackaged for msys2 — separate item). **Bonus:** the PR's macOS job surfaced that `tests/integration` had been RED on main (2 stale tests not updated for the conformance gate + octave-floored downsample) — both fixed in the same PR, main green again. | `ci.yml`, `tests/integration/` | [confirmed firsthand] | M | Med | | D5 | ~~**dciodvfy not in CI**~~ **DONE** (branch `ci/d5-dciodvfy`) — the macOS CI job now downloads a pinned `dicom3tools` macexe snapshot (cached) and runs `make dicom-validate`, which converts the JPEG / JPEG 2000 / HTJ2K fixtures **and** the A4b LZW→JPEG re-encode to WSM and validates every emitted instance with `dciodvfy` (exits non-zero only on errors). DICOM conformance is now gated on every push/PR. | `ci.yml`, `Makefile` | [confirmed firsthand] | S–M | Med | | D6 | ~~**CI `-timeout 5m`** vs heavy `-race cmd/wsitools` → false-FAIL risk under load~~ **DONE** (branch `ci/d6-test-timeout`) — bumped the unit `go test -race` step to `-timeout 30m`, matching the integration step + CLAUDE.md. | `ci.yml` | [confirmed firsthand] | S | Low | | D7 | ~~**No cross-implementation conformance check vs `wsidicomizer`**~~ **DONE** (study `docs/d7-dicom-conformance-vs-wsidicomizer-2026-06-20.md`) — diffed `convert --to dicom` vs **wsidicomizer 0.26.1** on CMU-1 (single-level) + 239551 (3-level) with pydicom. **Verdict: strong structural conformance** — every load-bearing WSM attribute matches (TILED_FULL, TotalPixelMatrix+origin, OpticalPath+ICC, Shared/Per-Frame FG, SpecimenDescription, AcquisitionContext, ImageType, TS, tile geom, frame counts, photometric). Two substantive divergences surfaced (both low-severity, dciodvfy-silent): **(a) PyramidUID** — reference links pyramid levels with a shared PyramidUID; wsitools omits it (by design, groups via shared Series+FrameOfRef — valid, but no modern explicit linkage). **(b) derived-level PixelSpacing** — wsitools per-axis-recomputes from rounded dims → slightly ANISOTROPIC at L1/L2; reference scales base×factor isotropic (~0.02% mag). Plus 2 cosmetic defaults (ImagedVolumeDepth 1.0 vs 0.5; ContainerIdentifier "WSITOOLS" vs "Unknown"). **Follow-up (a) PyramidUID DONE** (branch `feat/dicom-pyramiduid`): shared PyramidUID (0008,0019) now stamped on all VOLUME instances, none on associated — matches reference; dciodvfy 0-err. (b) PixelSpacing left as-is (NOT a defect — wsitools' per-axis spacing keeps `PixelSpacing × TotalPixelMatrix = ImagedVolumeWidth` exact per level; the reference's isotropic base×factor is the looser one). | `internal/dicomwriter/` | [confirmed firsthand] | M | Med–High | diff --git a/tests/integration/convert_codec_test.go b/tests/integration/convert_codec_test.go index 56a79d9..d55d763 100644 --- a/tests/integration/convert_codec_test.go +++ b/tests/integration/convert_codec_test.go @@ -17,11 +17,16 @@ import ( var novelCodecs = []struct { name string want opentile.Compression + // allowNonconformant marks codecs the conformance gate rejects for the TIFF + // container (writable bytes opentile-go can't read back) — they need + // --allow-nonconformant. Per containerCapabilities("tiff"), jpegxl is the + // lone non-conformant novel codec; avif/webp/htj2k are conformant. + allowNonconformant bool }{ - {"jpegxl", opentile.CompressionJPEGXL}, - {"avif", opentile.CompressionAVIF}, - {"webp", opentile.CompressionWebP}, - {"htj2k", opentile.CompressionHTJ2K}, + {"jpegxl", opentile.CompressionJPEGXL, true}, + {"avif", opentile.CompressionAVIF, false}, + {"webp", opentile.CompressionWebP, false}, + {"htj2k", opentile.CompressionHTJ2K, false}, } // TestConvertTIFF_NovelCodecs re-encodes CMU-1-Small-Region.svs to generic-TIFF @@ -54,7 +59,12 @@ func TestConvertTIFF_NovelCodecs(t *testing.T) { c := c t.Run(c.name, func(t *testing.T) { out := filepath.Join(t.TempDir(), "out.tiff") - cmd := exec.Command(bin, "convert", "--to", "tiff", "--codec", c.name, "-o", out, src) + args := []string{"convert", "--to", "tiff", "--codec", c.name} + if c.allowNonconformant { + args = append(args, "--allow-nonconformant") + } + args = append(args, "-o", out, src) + cmd := exec.Command(bin, args...) if b, err := cmd.CombinedOutput(); err != nil { t.Fatalf("convert --to tiff --codec %s: %v\n%s", c.name, err, b) } diff --git a/tests/integration/downsample_test.go b/tests/integration/downsample_test.go index ca9b13e..6bf3e61 100644 --- a/tests/integration/downsample_test.go +++ b/tests/integration/downsample_test.go @@ -64,8 +64,22 @@ func TestDownsample_CMU1SmallRegion(t *testing.T) { if string(outTlr.Format()) != "svs" { t.Errorf("output format: got %q, want svs", outTlr.Format()) } - if got, want := len(outTlr.Levels()), len(srcTlr.Levels()); got != want { - t.Errorf("level count: got %d, want %d", got, want) + // downsample emits an octave pyramid from the reduced L0 (retile engine): + // the output level count derives from the new L0, NOT the source's level + // count. Verify the pyramid is octave-structured — each level is the + // previous level halved with ceiling rounding, ceil(n/2) == (n+1)/2 (the + // engine's octave math) — rather than matching the source's count. + outLevels := outTlr.Levels() + if len(outLevels) < 1 { + t.Fatalf("output has no levels") + } + for i := 1; i < len(outLevels); i++ { + pw, ph := outLevels[i-1].Size.W, outLevels[i-1].Size.H + wantW, wantH := (pw+1)/2, (ph+1)/2 + w, h := outLevels[i].Size.W, outLevels[i].Size.H + if w != wantW || h != wantH { + t.Errorf("level %d not an octave of level %d: got %dx%d, want %dx%d", i, i-1, w, h, wantW, wantH) + } } srcL0, outL0 := srcTlr.Levels()[0], outTlr.Levels()[0]