Reuse ci.yml artifact for Falcor tests; remove standalone Falcor build+test workflows

[jkwak-work]

Fixes #11600

Motivation

The previous version of this PR removed the test/perf-test duplication between
falcor-test.yml and falcor-compiler-perf-test.yml by unifying them into
falcor-slang-test.yml, but falcor-slang-build.yml still built its own
Windows release of Slang on the [Windows, self-hosted, build] runner:

runs-on: [Windows, self-hosted, build]
...
cmake --preset default --fresh \
  -DSLANG_SLANG_LLVM_FLAVOR=USE_SYSTEM_LLVM \
  -DCMAKE_COMPILE_WARNING_AS_ERROR=true \
  -DSLANG_ENABLE_CUDA=1 \
  -DSLANG_ENABLE_EXAMPLES=0 \
  -DSLANG_ENABLE_GFX=0 \
  -DSLANG_ENABLE_TESTS=0 \
  -DSLANG_EXCLUDE_DAWN=1 \
  -DSLANG_EXCLUDE_TINT=1 \
  -DSLANG_ENABLE_SLANG_RHI=0

This is a second full Windows release build of Slang on the same
[Windows, self-hosted, build] runner pool that ci.yml already uses for
build-windows-release-cl-x86_64-gpu (via ci-slang-build.yml) on every PR.
That job already produces and uploads slang-tests-windows-x86_64-cl-release
— a bin/ directory with slangc.exe, the runtime DLLs, D3D12/, optix/
and the slang-standard-module-* directories, built with CUDA enabled (CUDA
is an auto_option that turns on when CUDAToolkit is found, which it is on
this runner pool — confirmed by common-test-setup's
check_api_support ... "cuda" check on test-windows-release-cl-x86_64-gpu).
That artifact is a strict superset of what the cut-down Falcor build produced.
Building it twice doubles the scarce [Windows, self-hosted, build] runner
time spent per PR for no functional benefit.

Proposed solution

Stop building a separate Slang binary for Falcor; instead have Falcor's test
jobs consume the slang-tests-windows-x86_64-cl-release artifact that
build-windows-release-cl-x86_64-gpu already produces, the same way
test-materialx-windows-release does today.

GitHub Actions artifacts are scoped to a single workflow run, so a job can only
actions/download-artifact something uploaded by another job in the same
run. falcor.yml and ci.yml were separate top-level workflows triggered
independently by the same pull_request event — two different runs — so
sharing the artifact "for free" via a plain needs: + download-artifact
requires the consuming jobs to live in ci.yml's own run. Concretely:

• .github/workflows/ci-falcor-test.yml (new) — reusable workflow containing
  test-falcor (unit + image tests, [Windows, self-hosted, falcor]) and
  test-falcor-perf (compiler perf test, [Windows, self-hosted, perf]),
  moved from falcor-slang-test.yml, now downloading
  slang-tests-windows-x86_64-cl-release instead of
  slang-falcor-build-windows-release.
• ci.yml gains a test-falcor job that calls ci-falcor-test.yml, with
  needs: [filter, build-windows-release-cl-x86_64-gpu] and an explicit
  github.event_name == 'pull_request' restriction to protect the scarce
  [Windows, self-hosted, falcor] and [Windows, self-hosted, perf] runners
  from merge_group or manual runs (unlike test-materialx-windows-release,
  which runs on all events after Require all CI needs to succeed #11619).
• test-falcor is added to check-ci's needs so Falcor results are gated
  by the same aggregator that gates the rest of CI. test-falcor runs on all
  events (no pull_request restriction), matching
  test-materialx-windows-release's behavior.
• falcor.yml, falcor-slang-build.yml and falcor-slang-test.yml are
  removed; their content is superseded by the jobs above.

Change summary

File	Change
.github/workflows/ci-falcor-test.yml	New. Reusable workflow_call containing test-falcor and test-falcor-perf, moved from falcor-slang-test.yml, now downloading slang-tests-windows-x86_64-cl-release.
.github/workflows/ci.yml	Adds test-falcor (uses: ./.github/workflows/ci-falcor-test.yml, needs: [filter, build-windows-release-cl-x86_64-gpu], runs on all events like test-materialx-windows-release) and adds test-falcor to check-ci.needs.
.github/workflows/falcor.yml	Removed. Its build → test → check-falcor shape is superseded by the jobs added to ci.yml.
.github/workflows/falcor-slang-build.yml	Removed. Falcor no longer builds its own Slang binary.
.github/workflows/falcor-slang-test.yml	Removed. Content moved to ci-falcor-test.yml with the artifact source updated.

Concepts and vocabulary

• slang-tests-${os}-${platform}-${compiler}-${config}: the artifact
  ci-slang-build.yml's "Prepare artifacts for upload" step uploads from
  artifacts-staging/${cmake_config}/, containing ${cmake_config}/bin
  (slangc + runtime DLLs + D3D12/ + optix/ + slang-standard-module-*),
  ${cmake_config}/lib, /share and /include. For
  os=windows, platform=x86_64, compiler=cl, config=release this is
  slang-tests-windows-x86_64-cl-release with ${cmake_config}=Release, so
  downloading it to a directory X yields X/Release/bin/... — the same
  artifact test-materialx-windows-release downloads via
  materialx-test.yml.
• check-ci: the single aggregator job in ci.yml that gates the full
  CI matrix, including Falcor's build+test results (test-falcor is in its
  needs). Uses strict success-only logic (fail on any result that is not
  success, with a filter-based early exit for doc-only diffs).

Process report

• Sharing build-windows-release-cl-x86_64-gpu's artifact instead of a
  dedicated Falcor build (ci-falcor-test.yml): the cut-down build in
  falcor-slang-build.yml (-DSLANG_ENABLE_GFX=0 -DSLANG_ENABLE_TESTS=0 -DSLANG_EXCLUDE_DAWN=1 -DSLANG_EXCLUDE_TINT=1 -DSLANG_ENABLE_SLANG_RHI=0,
  but -DSLANG_ENABLE_CUDA=1) was already shown (previous revision of this PR)
  to be sufficient for both test-falcor and test-falcor-perf — neither
  touches slang-test, slang-rhi-tests, GFX examples, Dawn, or Tint.
  ci-slang-build.yml's default config for build-windows-release-cl-x86_64-gpu
  builds all of those plus CUDA (CUDA is an auto_option that defaults on
  when CUDAToolkit is found, which common-test-setup's
  check_api_support ... "cuda" step confirms is true for this runner pool via
  test-windows-release-cl-x86_64-gpu). So
  slang-tests-windows-x86_64-cl-release's Release/bin/ is a strict superset
  of the old slang-falcor-build-windows-release artifact's contents: same
  slangc.exe + runtime DLLs + D3D12//optix//slang-standard-module-*,
  plus extra binaries (slang-test.exe, slang-rhi-tests.exe, GFX/Dawn/Tint
  DLLs) that Falcor's tests never invoke. Reusing it removes an entire
  duplicate Windows release build from the critical path of every PR.

• test-falcor/test-falcor-perf must live inside ci.yml's job graph, not
  a separate falcor.yml (ci.yml, removal of falcor.yml): GitHub Actions
  artifacts uploaded via actions/upload-artifact are scoped to the workflow
  run that produced them; a job in a different workflow run (even for the
  same commit) cannot actions/download-artifact it without a separate
  run-id-based cross-run lookup (finding the matching ci.yml run for the
  same SHA, which is racy and adds a class of failure this refactor doesn't
  need). The only way to consume
  build-windows-release-cl-x86_64-gpu's artifact via a plain needs: +
  download-artifact — as test-materialx-windows-release already does — is
  for the consuming job to be part of the same ci.yml run. falcor.yml's
  build/test/check-falcor jobs are therefore folded into ci.yml as
  test-falcor/check-falcor, and falcor.yml is deleted as redundant.

• test-falcor runs on all events; gated by check-ci (ci.yml): the
  earlier revision of this PR restricted test-falcor to pull_request events
  to protect the scarce [Windows, self-hosted, falcor] and
  [Windows, self-hosted, perf] runners from merge_group. Per reviewer
  feedback, this restriction is removed so that test-falcor runs on all
  events exactly like test-materialx-windows-release, and check-ci can gate
  its result uniformly without any special-casing. test-falcor is added
  directly to check-ci.needs; the separate check-falcor aggregator is
  removed. The skipped-dependency gap from Gate check-ci on build job results so broken builds cannot merge #11585 is handled by check-ci's
  existing generic toJSON(needs) + jq pass: build-windows-release-cl-x86_64-gpu
  is already in check-ci.needs, so a build failure is caught directly even if
  test-falcor only reports skipped.

• Artifact layout and selective-copy change inside ci-falcor-test.yml:
  the old slang-falcor-build-windows-release artifact had path: build/Release/bin/, i.e. the bin/ contents sat at the artifact root, and
  it was a reduced build (-DSLANG_ENABLE_GFX=0 -DSLANG_EXCLUDE_DAWN=1 -DSLANG_EXCLUDE_TINT=1) that contained only Slang core DLLs with no
  gfx.dll, platform.dll, dawn.dll, or test-tool DLLs — and no D3D12/
  or optix/ subdirectories (those only appear when Dawn is enabled). The new
  slang-tests-windows-x86_64-cl-release artifact is a full CI build; its
  root is Release/bin/ (one extra path component) and it also contains
  gfx.dll, platform.dll, dawn.dll, webgpu_dawn.dll, test/example
  binaries, and — critically — D3D12/D3D12Core.dll, D3D12/d3d12SDKLayers.dll,
  and optix/ headers. Two steps are adjusted:

  • test-falcor's "Copy Slang to Falcor" step now selectively copies only
    the Slang core DLLs (slang.dll, slang-rt.dll, slang-compiler.dll,
    slang-llvm.dll, slang-glslang.dll, slang-glsl-module.dll,
    dxcompiler.dll, dxil.dll, slangc.exe) and slang-standard-module-*/
    directories. D3D12/ and optix/ are intentionally NOT copied: the old
    Falcor-specific build never produced them (it excluded Dawn), so setup-falcor
    already provides the correct D3D12Core.dll version from C:\Falcor.
    Overwriting it with the CI artifact's D3D12 Agility SDK version causes
    FalcorTest.exe to exit immediately on startup. slang-rt.dll must be
    copied because it is a version-matched runtime peer of slang.dll — the old
    reduced Falcor build produced it in the same artifact, and omitting it while
    replacing slang.dll leaves C:\Falcor's older slang-rt.dll in place,
    which is incompatible with the CI-built slang.dll and causes FalcorTest.exe
    to crash during DLL initialization.
  • test-falcor-perf's download path: changes from build/Release/bin to
    build, so the artifact's own Release/bin/... lands at
    build/Release/bin/... — exactly what the existing $env:PATH = ".\build\Release\bin;..." line already expects, so that line is unchanged.

• Removed falcor.yml's standalone workflow_dispatch: the previous
  revision allowed manually re-running just the Falcor workflow without the
  rest of ci.yml's matrix. A standalone dispatch of a separate workflow
  cannot consume an artifact from a specific ci.yml run (the artifact is
  scoped to that run), so preserving a separate dispatch entry point would
  require either re-introducing the duplicate build this PR removes, or a
  cross-run artifact lookup. Re-running Falcor's tests now means re-running
  (or re-dispatching) ci.yml, which is consistent with how
  test-materialx-windows-release is already re-run today.

• Bash-only steps in ci-falcor-test.yml (per reviewer feedback): all
  steps that previously used shell: pwsh — setup-falcor, falcor-unit-test,
  falcor-image-test, and run falcor-compiler-perf-test — have been converted
  to bash. The Falcor runner machines require Git Bash on PATH (the runners do
  not add it by default), so the two Add Git Bash to PATH steps (one per job)
  still use shell: pwsh to write to $env:GITHUB_PATH via Add-Content, as
  that is the required bootstrap pattern. All subsequent steps use the job-level
  default shell: bash: Copy-Item/mkdir/cd in setup-falcor becomes
  cp -r/mkdir -p/find … -delete; $ErrorActionPreference = "Stop" +
  explicit $LASTEXITCODE checks become set -euo pipefail; Expand-Archive
  in the perf test becomes python -m zipfile -e; and $env:PATH = "…;" + …
  becomes export PATH="…:$PATH".

• Least-privilege permissions: on the new Falcor jobs
  (ci-falcor-test.yml, ci.yml:test-falcor): per CodeRabbit review
  feedback, the new jobs are given explicit permissions: instead of relying
  on the repository's default GITHUB_TOKEN scopes, following the precedent
  already set in this file by retry-on-gpu-failure (permissions: {actions: write}), and by ci-slang-build.yml's build job (permissions: {id-token: write, contents: read}). ci-falcor-test.yml gets a
  workflow-level permissions: {contents: read, actions: read} — test-falcor
  and test-falcor-perf only run actions/checkout and
  actions/download-artifact/robinraju/release-downloader, neither of which
  needs write access. ci.yml's test-falcor job, which calls
  ci-falcor-test.yml via uses:, declares the same read-only scope so the
  reusable workflow cannot be granted broader permissions than it needs by its
  caller.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 882e408a-a30d-48b6-b084-b78e6a5b0b36

📥 Commits

Reviewing files that changed from the base of the PR and between d277532 and 15fc90f.

📒 Files selected for processing (1)

• .github/workflows/ci-falcor-test.yml

---

📝 Walkthrough

Walkthrough

Removes the standalone falcor-test.yml workflow. Converts ci-falcor-test.yml from a PR-triggered perf workflow into a reusable workflow_call workflow with a test-falcor job (unit and image tests) and a test-falcor-perf job. Adds test-falcor job to ci.yml and updates the check-ci aggregate gate to include it in the main CI pipeline.

Changes

Falcor CI Refactor

Layer / File(s)	Summary
ci-falcor-test.yml as reusable workflow with test-falcor job .github/workflows/ci-falcor-test.yml	Converts workflow trigger to workflow_call with restricted contents: read and actions: read permissions. The test-falcor job runs on the falcor self-hosted Windows runner: performs shallow/no-submodules checkout, downloads slang-tests-windows-x86_64-cl-release artifact, stages Falcor test/media/tools/scripts directories via PowerShell, copies Slang Release binaries and standard modules into Falcor release bin with file existence checks, then runs run_unit_tests.py and run_image_tests.py with explicit exit-code validation.
test-falcor-perf job in ci-falcor-test.yml .github/workflows/ci-falcor-test.yml	Defines the test-falcor-perf job on the perf self-hosted runner: performs shallow/no-submodules checkout, downloads slang-tests-windows-x86_64-cl-release artifact into build/, downloads the falcor_perf_test-*-win-64.zip release, extracts it, updates PATH to use Slang binaries from build\Release\bin, and runs falcor_perftest.exe with exit-code failure propagation.
test-falcor job and check-ci gate in ci.yml .github/workflows/ci.yml	Adds test-falcor job that runs only when filter.outputs.should-run == 'true', depends on build-windows-release-cl-x86_64-gpu, and invokes the reusable ./.github/workflows/ci-falcor-test.yml workflow with read-only contents: read and actions: read permissions. Updates the existing check-ci aggregate gate to include test-falcor in its needs list, making the aggregate CI gate incorporate the Falcor test job outcome.

Suggested reviewers

• bmillsNV
• jkiviluoto-nv

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: reusing the ci.yml artifact for Falcor tests and removing standalone Falcor build+test workflows, which is the core objective of this PR.
Description check	✅ Passed	The description is comprehensive and directly related to the changeset, explaining motivation, proposed solution, file changes, concepts, and detailed process reports for all modifications.
Linked Issues check	✅ Passed	The PR substantially achieves #11600's objectives through a more efficient architecture: consolidates Falcor tests into ci.yml with reusable ci-falcor-test.yml, eliminates duplicate builds by reusing ci-slang-build.yml artifact, and provides unified gating via check-ci instead of separate check-falcor.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the linked issue #11600 requirements: creating ci-falcor-test.yml, integrating test jobs into ci.yml, and removing falcor.yml/falcor-slang-build.yml/falcor-slang-test.yml. No unrelated modifications found.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d52b21d6-9ee4-44c7-a3a1-6eec55f77a52

📥 Commits

Reviewing files that changed from the base of the PR and between a68b606 and 049a861.

📒 Files selected for processing (4)

• .github/workflows/falcor-slang-build.yml
• .github/workflows/falcor-slang-test.yml
• .github/workflows/falcor-test.yml
• .github/workflows/falcor.yml

💤 Files with no reviewable changes (1)

• .github/workflows/falcor-test.yml

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

.github/workflows/ci-falcor-test.yml (1)

3-5: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Declare least-privilege token permissions for this reusable workflow.

Line 3 introduces a reusable entrypoint without explicit permissions, so jobs inherit default token scope. On self-hosted runners, this is an avoidable security exposure.

Suggested fix

 on:
   workflow_call:
+
+permissions:
+  contents: read
+  actions: read

As per coding guidelines, “.github/workflows/**: CI/CD workflow changes. Verify correct trigger conditions, proper secret handling, and that matrix configurations cover all required platforms.”

Sources: Coding guidelines, Linters/SAST tools

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e50f1af8-db57-48ac-af75-365bf0a6151b

📥 Commits

Reviewing files that changed from the base of the PR and between 435fa40 and 65aad9d.

📒 Files selected for processing (2)

• .github/workflows/ci-falcor-test.yml
• .github/workflows/ci.yml

[coderabbitai]

Actionable comments posted: 2

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6c40c6ed-78cc-47ee-bc33-9a1a56e24584

📥 Commits

Reviewing files that changed from the base of the PR and between 65aad9d and 01065f2.

📒 Files selected for processing (3)

• .github/workflows/ci-falcor-test.yml
• .github/workflows/ci.yml
• .github/workflows/falcor-test.yml

💤 Files with no reviewable changes (1)

• .github/workflows/falcor-test.yml

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c5b43493-b0d7-4c27-99bd-a95c1dedea25

📥 Commits

Reviewing files that changed from the base of the PR and between 65aad9d and a4c2c48.

📒 Files selected for processing (3)

• .github/workflows/ci-falcor-test.yml
• .github/workflows/ci.yml
• .github/workflows/falcor-test.yml

💤 Files with no reviewable changes (1)

• .github/workflows/falcor-test.yml

[coderabbitai]

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c5b43493-b0d7-4c27-99bd-a95c1dedea25

📥 Commits

Reviewing files that changed from the base of the PR and between 65aad9d and a4c2c48.

📒 Files selected for processing (3)

• .github/workflows/ci-falcor-test.yml
• .github/workflows/ci.yml
• .github/workflows/falcor-test.yml

💤 Files with no reviewable changes (1)

• .github/workflows/falcor-test.yml

🛑 Comments failed to post (1)

.github/workflows/ci.yml (1)

71-71: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Pin actions/checkout to a commit SHA in wait-for-human-priority.

Line 71 uses a mutable tag (@v4), which weakens workflow integrity compared with the repository’s pinned-action pattern.

Suggested fix

-      - uses: actions/checkout@v4
+      - uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5

As per coding guidelines, “.github/workflows/**: CI/CD workflow changes. Verify correct trigger conditions, proper secret handling, and that matrix configurations cover all required platforms.”

🧰 Tools

🪛 zizmor (1.25.2)

[error] 71-71: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

Sources: Coding guidelines, Linters/SAST tools

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

.github/workflows/ci-falcor-test.yml (1)

132-143: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Template expansion in shell command could be hardened.

The static analysis tool flags line 135 for potential template injection. While the risk is low because the filename originates from a controlled repository's releases (shader-slang/falcor-compile-perf-test), directly expanding ${{ ... }} inside a shell script can be fragile if the filename ever contains shell metacharacters.

Using an environment variable intermediary is the safer pattern used elsewhere in GitHub Actions:

Suggested hardening

       - name: run falcor-compiler-perf-test
+        env:
+          DOWNLOADED_FILE: ${{ fromJson(steps.download.outputs.downloaded_files)[0] }}
         run: |
           set -euo pipefail
-          filename='${{ fromJson(steps.download.outputs.downloaded_files)[0] }}'
+          filename="$DOWNLOADED_FILE"
           [[ -f "$filename" ]] || { echo "Downloaded file not found: $filename" >&2; exit 1; }
           python -m zipfile -e "$filename" ./falcor-perf-test
           export PATH="./build/Release/bin:$PATH"
           echo "PATH is $PATH"
           echo "Running falcor performance test..."
           ./falcor-perf-test/bin/Release/falcor_perftest.exe
           echo "falcor performance test completed successfully"

Source: Linters/SAST tools

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c01bdd2b-630a-4f0a-a96e-368510a600ec

📥 Commits

Reviewing files that changed from the base of the PR and between 78c8872 and d277532.

📒 Files selected for processing (1)

• .github/workflows/ci-falcor-test.yml

[jkwak-work]

Addressed the template expansion hardening from the latest CodeRabbit review: the ${{ fromJson(...) }} expression in run falcor-compiler-perf-test is now passed through an env: variable (DOWNLOADED_FILE) rather than expanded directly inside the shell script. Commit 15fc90f.

[jkwak-work]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[jkwak-work]

Looks good to me