Skip to content

fix(test): make the Metal self-tests build on MSVC, pass on macOS and fit the sanitizer job - #1925

Merged
lusoris merged 1 commit into
masterfrom
fix/metal-selftests-ci
Oct 3, 2026
Merged

lusoris merged 1 commit into
masterfrom
fix/metal-selftests-ci

Conversation

@lusoris

@lusoris lusoris commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

Summary

This fixes three hosted CI breaks that #1918 introduced when it built the Metal parity tests as self-tests on every host. The changes are tests only; no library code changes.

  • Windows MSVC: two tests named a scenario SC_DEFAULT, which winuser.h defines as 0xF160, so the build failed with C2059. The scenarios are now SC_DEFAULTS.
  • macOS clang: test_metal_selftest_integer_psnr crashed with SIGSEGV while writing the JSON output to a temporary file and reading apsnr_* back. The test now reads the aggregate straight from the context's feature collector.
  • Sanitizers: test_float_moment_sum (ADR-1497) hit its 120 s timeout under the debug ASan + UBSan build. Its timeout is now 600 s. The job did not produce an empty test list; the log only shows the workflow's script text, which contains that message.
  • Ubuntu ARM clang / macOS (aarch64): test_metal_selftest_float_moment fails because NEON and SVE2 add the second moment in lane order. fix(simd): add the NEON and SVE2 float_moment lanes in the scalar's order so aarch64 returns the scalar bits past 2^53 units #1923 fixes that, and this PR changes nothing for it.

Type

  • test — test-only

Checklist

  • Commits follow Conventional Commits (the commit-msg hook enforces this).
  • make format && make lint is green locally. The pre-commit hooks pass, and scripts/dev/preflight.sh --stage msvcism passes on the touched files.
  • Unit tests pass. I ran the sanitizer job's own invocation locally: the same meson setup flags, the same introspect-and-exclude list, and run_meson_test.py -- -C build --print-errorlogs $TESTS.
    • It enumerated 293 tests, 287 passed and 2 were skipped.
    • All 18 test_metal_selftest_* tests passed, and test_float_moment_sum passed in 35 s.
    • Four DNN tests failed only because this host's ONNX Runtime differs. The hosted job has no ONNX Runtime and does not run them.
  • If I touched any SIMD/GPU code path, I ran /cross-backend-diff and the worst ULP is ≤ 2. — Not applicable: only tests changed.
  • If I touched a feature extractor with SIMD/GPU twins, I either updated every twin or listed the gap under "Known follow-ups" below.
  • If I added a new .c / .cpp / .cu / .h / .hpp, it has the appropriate license header (see CONTRIBUTING.md).
  • If this is a breaking change, the commit message uses ! or BREAKING CHANGE: and the migration path is documented below. — Not breaking.
  • If this PR adds an ADR, the ADR row lives in docs/adr/_index_fragments/<NNNN-slug>.md and the slug is appended to docs/adr/_index_fragments/_order.txt. — No ADR.

Bug-status hygiene (ADR-0165)

  • docs/state.md: T-METAL-SELFTESTS-HOSTED-CI-2026-10-03 is under Recently closed.

Netflix golden-data gate (ADR-0024)

  • I did not modify any assertAlmostEqual(...) score in the Netflix golden Python tests.

Deep-dive deliverables (ADR-0108)

  • Research digest — no digest needed: test-only CI repair.
  • Decision matrix — no alternatives: only-one-way fix (rename the colliding identifier, use the existing test accessor, size the timeout).
  • AGENTS.md invariant note — core/test/AGENTS.d/metal-parity-tests.md: no identifiers that windows.h defines, the MinGW check command, and aggregates read through the collector accessor.
  • Reproducer / smoke-test command — below.
  • CHANGELOG fragment — no changelog: test-only, with no user-visible change.
  • Rebase note — no rebase impact: test files only.

Reproducer

# MSVC's C2059, reproduced with MinGW. Run it on the parent commit to see the error, and here to see it gone:
x86_64-w64-mingw32-gcc -fsyntax-only -std=gnu11 -include windows.h -DVMAF_METAL_TWIN_SELFTEST \
  -Icore/include -Icore/src -Icore/test -I<build>/include -I<build>/src core/test/test_metal_integer_motion_parity.c
# The sanitizer job, locally:
cd core && meson setup build-asan -Db_sanitize=address -Dc_args="-fsanitize=undefined -fno-sanitize=function" \
  -Dcpp_args="-fsanitize=undefined -fno-sanitize=function" -Denable_cuda=false -Denable_sycl=false \
  --buildtype=debug -Db_lto=false -Db_lundef=false && meson compile -C build-asan
ASAN_OPTIONS=halt_on_error=1:detect_leaks=1 ./build-asan/test/test_metal_selftest_integer_psnr

Known follow-ups

  • The macOS SIGSEGV happened inside the JSON round trip. It does not reproduce on Linux, under ASan or UBSan or without them, so I did not isolate the exact fault. The hosted macOS job on this PR will show whether the new collector read passes there.

… fit the sanitizer job (#1925)

* fix(test): make the Metal self-tests build on MSVC, pass on macOS and fit the sanitizer job

#1918 put the Metal parity tests on every host as self-tests, and three
hosted jobs failed on them.

MSVC: test_metal_integer_motion_parity.c and test_metal_motion_v2_parity.c
named a scenario SC_DEFAULT, which winuser.h defines as 0xF160; the pthread
shim includes windows.h, so the definition read `static const Scenario
0xF160 = {` (C2059). The scenarios are SC_DEFAULTS. A MinGW
`-fsyntax-only -include windows.h` check reproduces the error on the old
files and is clean on every test_metal_*_parity.c now.

macOS: test_metal_selftest_integer_psnr died with SIGSEGV in
expect_aggregate() on the hosted runner (release build with LTO), in the
path that wrote the context's JSON output to a mkstemp() file and read
apsnr_* back. The test passes on Linux under ASan and UBSan, so the fault
inside that path was not isolated here. The test now reads the aggregate
from the context's feature collector (vmaf_feature_collector_get() of
libvmaf_priv.h and vmaf_feature_collector_get_aggregate()), the double the
writer prints, with no file, temporary directory or windows.h.

Sanitizers: the job failed on test_float_moment_sum (ADR-1497) reaching its
120 s timeout under the debug ASan + UBSan build on the hosted runner, twice.
It takes 35 s on a workstation in that configuration; the timeout is 600 s.
The job's own invocation, run here, enumerates 293 tests and passes every
Metal self-test.
@github-actions github-actions Bot added the type:bug Something isn't working label Oct 3, 2026
@lusoris
lusoris force-pushed the fix/metal-selftests-ci branch from df55def to 9cf0842 Compare October 3, 2026 15:09
@lusoris
lusoris merged commit 9cf0842 into master Oct 3, 2026
43 of 44 checks passed
@lusoris
lusoris deleted the fix/metal-selftests-ci branch October 3, 2026 15:09
lusoris added a commit that referenced this pull request Oct 3, 2026
…st_float_moment_sum

The hosted ASan + UBSan job killed test_metal_float_moment_sum at 120 s, as it killed test_float_moment_sum before #1925. Both run the same past-2^53 frames and take 35 s on a workstation under that build; the Metal mirror now has the same 600 s timeout.
lusoris added a commit that referenced this pull request Oct 3, 2026
…st_float_moment_sum

The hosted ASan + UBSan job killed test_metal_float_moment_sum at 120 s, as it killed test_float_moment_sum before #1925. Both run the same past-2^53 frames and take 35 s on a workstation under that build; the Metal mirror now has the same 600 s timeout.
lusoris added a commit that referenced this pull request Oct 3, 2026
…st_float_moment_sum

The hosted ASan + UBSan job killed test_metal_float_moment_sum at 120 s, as it killed test_float_moment_sum before #1925. Both run the same past-2^53 frames and take 35 s on a workstation under that build; the Metal mirror now has the same 600 s timeout.
lusoris added a commit that referenced this pull request Oct 4, 2026
…st_float_moment_sum

The hosted ASan + UBSan job killed test_metal_float_moment_sum at 120 s, as it killed test_float_moment_sum before #1925. Both run the same past-2^53 frames and take 35 s on a workstation under that build; the Metal mirror now has the same 600 s timeout.
lusoris added a commit that referenced this pull request Oct 4, 2026
…st_float_moment_sum

The hosted ASan + UBSan job killed test_metal_float_moment_sum at 120 s, as it killed test_float_moment_sum before #1925. Both run the same past-2^53 frames and take 35 s on a workstation under that build; the Metal mirror now has the same 600 s timeout.
lusoris added a commit that referenced this pull request Oct 4, 2026
…st_float_moment_sum

The hosted ASan + UBSan job killed test_metal_float_moment_sum at 120 s, as it killed test_float_moment_sum before #1925. Both run the same past-2^53 frames and take 35 s on a workstation under that build; the Metal mirror now has the same 600 s timeout.
lusoris added a commit that referenced this pull request Oct 4, 2026
…st_float_moment_sum

The hosted ASan + UBSan job killed test_metal_float_moment_sum at 120 s, as it killed test_float_moment_sum before #1925. Both run the same past-2^53 frames and take 35 s on a workstation under that build; the Metal mirror now has the same 600 s timeout.
lusoris added a commit that referenced this pull request Oct 4, 2026
…st_float_moment_sum

The hosted ASan + UBSan job killed test_metal_float_moment_sum at 120 s, as it killed test_float_moment_sum before #1925. Both run the same past-2^53 frames and take 35 s on a workstation under that build; the Metal mirror now has the same 600 s timeout.

This branch was successfully deployed

No deployments
github-pages — 9cf0842c Deployed Oct 3, 2026 by lusoris via deploy #4238
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant