Skip to content

coverage CI: mark SlangcCoverageManifestOutput as expected failure on macOS #11630

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
skiminki-nv merged 1 commit into from
Jun 16, 2026
+9 −0
Merged

coverage CI: mark SlangcCoverageManifestOutput as expected failure on macOS #11630

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions tests/expected-failure-coverage.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,12 @@
# escalates that failure into a SIGSEGV inside the slang-test
# orchestrator, which kills the rest of the suite.
docs/generated/tests/design/ir-reference/metadata/unorm-attr-on-buffer-element.slang

# Coverage metadata is not propagated from the upstream Slang IR artifact
# through the Metal downstream compilation chain (xcrun metallib-asm path),
# so _maybeWriteCoverageManifest finds no ICoverageTracingMetadata on the
# final artifact and writes no manifest file. The two MetalLib sub-checks
# inside this unit test therefore always fail on macOS coverage CI where the
# Metal toolchain is present. Tracked in: https://github.com/shader-slang/slang/issues/11629
# On non-Apple CI the sub-checks are skipped via _isMetalLibDownstreamUnavailable.

@github-actions github-actions Bot Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Gap: Comment misidentifies the non-Apple skip mechanism

The line says: # On non-Apple CI the sub-checks are skipped via _isMetalLibDownstreamUnavailable. In source, both Metal sub-checks at tools/slang-unit-test/unit-test-stdin-compile.cpp:778 (_testCoverageAutoSidecarForMetalLibDisassembly) and :852 (_testCoverageExplicitSidecarForMetalLibDisassembly) are wrapped in #if SLANG_APPLE_FAMILY with an #else return SLANG_OK; branch (around lines 800–803 and 876–879). On non-Apple builds the entire body is compiled out — _isMetalLibDownstreamUnavailable is never reached. That helper only handles the runtime case where the Metal toolchain is missing on Apple.

Suggestion: Reword to accurately credit the preprocessor guard:

# On non-Apple builds the sub-checks are compiled out via `#if SLANG_APPLE_FAMILY`;
# on Apple platforms without a Metal toolchain they return early via
# `_isMetalLibDownstreamUnavailable` (E52002 "pass-through compiler not found").

slang-unit-test-tool/SlangcCoverageManifestOutput.internal

@github-actions github-actions Bot Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Gap: Expected-failure granularity masks future regressions in 25 unrelated sub-checks

expected-failure-coverage.txt operates at unit-test granularity, but SLANG_UNIT_TEST(SlangcCoverageManifestOutput) at tools/slang-unit-test/unit-test-stdin-compile.cpp:1610 aggregates 27 SLANG_CHECK calls. Only sub-checks 3 and 6 — _testCoverageAutoSidecarForMetalLibDisassembly (line 778) and _testCoverageExplicitSidecarForMetalLibDisassembly (line 852) — are Metal-related. The other 25 cover auto-sidecar, explicit-sidecar, stdout artifact, overwrite/symlink/case-alias collisions, multi-entry, whole-program, unsupported-target rejection, write-failure reporting, and compiler-option-hash invariance. With this entry, a future regression in any of those 25 that only manifests under LLVM coverage instrumentation on macOS will be reclassified as "expected" and ship undetected on that lane (Linux/Windows non-coverage and macOS non-coverage CI would still catch it, so the blind spot is genuinely macOS-coverage-only — but the file's own header documents that LLVM-coverage-only failure modes do exist).

Example: If a refactor of the sidecar-overwrite logic introduces a regression that only repros under LLVM coverage on macOS, the unit test fails, the runner reclassifies it as expected, the regression ships, and is only caught when #11629 is fixed and this entry is removed.

Suggested fix: Guard the two MetalLib sub-checks at the test level rather than skipping the whole test. For example, skip lines 1614 and 1617–1618 (the two SLANG_CHECK(_testCoverageExplicit/AutoSidecarForMetalLibDisassembly(...)) invocations) when running under coverage instrumentation on macOS — via a runtime probe in those helpers, an env-var, or a coverage-build define. Then drop this expected-failure-coverage.txt entry. That preserves regression coverage for the 25 non-Metal sub-checks until #11629 is fixed.

Loading