test(shader): fallback to warp on invalidarg

[LukeFrankio]

Summary

• Add a robust fallback path in shader runtime test discovery for systems where hardware D3D12 initialization fails with HRESULT 0x80070057 (E_INVALIDARG).
• Preserve hardware execution as the primary path, but automatically retry with software WARP when the specific initialization failure is encountered.
• Keep test output semantics unchanged while reducing environment-specific false negatives.

What changed

tests/shaders/test_common.h

• Updated GetFixtureDesc(...) to accept an explicit stf::GPUDevice::EDeviceType parameter.
• Kept current behavior intact by defaulting to Hardware.

tests/shaders/runtime_test_discovery.h

• Added a preferred-device state machine (Undecided, Hardware, Software).
• Introduced a local execution helper that runs a test using a selected device type.
• Implemented one-time probing logic:
  1. Try hardware first.
  2. If initialization throws stf::HrException with E_INVALIDARG, retry using software WARP.
  3. Cache the successful path for remaining discovered tests in the same process.
• Preserved existing failure-detail extraction and reporting behavior.

Why this change

The previous harness always initialized hardware for every discovered test. On systems with incompatible D3D12 hardware/runtime combinations, this can fail before shader assertions run, causing repeated suite failures that do not reflect shader correctness. This update keeps the fast path when available and introduces a deterministic, minimal fallback when hardware initialization is invalid.

Validation

• Built and ran run_shader_tests.
• Observed expected fallback behavior:
  • hardware path reports E_INVALIDARG
  • fallback to software WARP is engaged
• Final result: All tests passed (164 assertions in 1 test case).

Scope

• Change is intentionally localized to shader test harness plumbing only.
• No production shader runtime path or gameplay runtime logic changed.

Summary by CodeRabbit

• New Features

  • Added a user-facing preferred-device setting to remember and expose Hardware vs. Software GPU choice.
  • Added ability to request test fixtures for a specific GPU device type.

• Tests

  • Implemented device-aware test execution with intelligent fallback: try preferred device, default to Hardware, and retry with Software on specific failures; logs and preserves error details.
  • Generated shader tests now explicitly target Hardware by default.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds device-aware test execution and fixture configuration: runtime_test_discovery now selects between Hardware and Software GPU devices with retry and caching; test_common exposes preferred-device state, conversion helpers, and parameterized fixture description; macros updated to pass explicit device type for fixtures.

Changes

Cohort / File(s)	Summary
Device-aware discovery tests/shaders/runtime_test_discovery.h	Refactored test runner to use runWithDevice(...), branch on EPreferredDevice, try Hardware then Software on E_INVALIDARG, cache preferred device, and preserve existing error extraction/reporting.
Preferred-device state & fixture helpers tests/shaders/test_common.h	Added ShaderTest::EPreferredDevice, inline accessors (GetPreferredDeviceState, SetPreferredDevice, GetPreferredDevice), conversion helpers to stf::GPUDevice::EDeviceType, and GetFixtureDesc(deviceType) plus a no-arg overload that uses preferred device.
Fixture usage in test generation tests/shaders/test_helpers_unified.h	Updated test macro and generation to pass GetFixtureDesc(stf::GPUDevice::EDeviceType::Hardware) explicitly, making generated tests target Hardware fixture by default.

Sequence Diagram

sequenceDiagram
    actor TestRunner as Test Runner
    participant Discovery as RuntimeDiscovery
    participant Hardware as HardwareDevice
    participant Software as SoftwareDevice

    TestRunner->>Discovery: runTest()
    alt preferred == Undecided
        Discovery->>Hardware: runWithDevice(Hardware)
        alt success
            Hardware-->>Discovery: success/result
            Discovery->>Discovery: SetPreferredDevice(Hardware)
        else E_INVALIDARG
            Hardware-->>Discovery: throws E_INVALIDARG
            Discovery->>Software: runWithDevice(Software)
            Software-->>Discovery: success/result
            Discovery->>Discovery: SetPreferredDevice(Software)
        end
    else preferred == Hardware
        Discovery->>Hardware: runWithDevice(Hardware)
        Hardware-->>Discovery: result
    else preferred == Software
        Discovery->>Software: runWithDevice(Software)
        Software-->>Discovery: result
    end
    Discovery-->>TestRunner: return result / errors

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• doodlum
• jiayev

Poem

🐰 I hopped into tests at dawn,

Tried hardware first, then software on,
If silicon shrugs with a cryptic cry,
I bounce to fallback and give it a try,
I stash the winner and nibble a prawn.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 38.46% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'test(shader): fallback to warp on invalidarg' is specific and clearly describes the main change—a fallback mechanism to WARP device on E_INVALIDARG errors in shader tests.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

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: 1

🧹 Nitpick comments (2)

tests/shaders/runtime_test_discovery.h (2)

18-23: PR metadata: tighten title length and add issue linkage if applicable.

Suggested <=50-char title: fix(shader-tests): fallback to warp on invalidarg
If this resolves a tracked bug, add Fixes #<id> in the PR description.

As per coding guidelines, "Conventional Commit Titles ... Length: 50 characters limit for title" and "Issue References ... 'Fixes #123' or 'Closes #123' for bug fixes."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/shaders/runtime_test_discovery.h` around lines 18 - 23, Update the PR
metadata: shorten the PR title to <=50 characters (suggested:
"fix(shader-tests): fallback to warp on invalidarg") and, if this change
resolves a tracked bug, add an issue reference in the PR description using the
Conventional Commit format (e.g., "Fixes #<id>"). No code changes required—just
adjust the PR title and description accordingly.

---

204-205: Consider centralizing preferred-device state for all shader test helpers.

preferredDevice is scoped to runTest, while other helper paths still construct fixtures using default hardware (tests/shaders/test_helpers_unified.h, Line [31] and Line [90]). Sharing this selection logic would avoid mixed device behavior within the same test process.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/shaders/runtime_test_discovery.h` around lines 204 - 205,
preferredDevice is currently declared inside runTest causing mixed device
selection across helpers; centralize this state by moving EPreferredDevice
preferredDevice into a single shared location (e.g. a static/inline variable or
accessor functions in the test helpers module) and update runTest and all
fixture constructors (those that currently default to hardware in the unified
test helpers) to read/write that centralized API (use names: EPreferredDevice,
preferredDevice, runTest, and the test helper fixture constructors) so every
helper observes the same selected device for the process.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/shaders/runtime_test_discovery.h`:
- Around line 232-238: The current fast-return when preferredDevice ==
EPreferredDevice::Hardware skips retrying on E_INVALIDARG; change the Hardware
branch so it invokes runWithDevice(stf::GPUDevice::EDeviceType::Hardware) but
does not return immediately — instead detect failure (HRESULT/exception) for
E_INVALIDARG and on that specific error call
runWithDevice(stf::GPUDevice::EDeviceType::Software) to perform the WARP
fallback; keep the Software branch behavior unchanged. Ensure you reference
preferredDevice, EPreferredDevice, runWithDevice, and
stf::GPUDevice::EDeviceType::Hardware/Software when making this change and only
treat E_INVALIDARG as the trigger for the fallback.

---

Nitpick comments:
In `@tests/shaders/runtime_test_discovery.h`:
- Around line 18-23: Update the PR metadata: shorten the PR title to <=50
characters (suggested: "fix(shader-tests): fallback to warp on invalidarg") and,
if this change resolves a tracked bug, add an issue reference in the PR
description using the Conventional Commit format (e.g., "Fixes #<id>"). No code
changes required—just adjust the PR title and description accordingly.
- Around line 204-205: preferredDevice is currently declared inside runTest
causing mixed device selection across helpers; centralize this state by moving
EPreferredDevice preferredDevice into a single shared location (e.g. a
static/inline variable or accessor functions in the test helpers module) and
update runTest and all fixture constructors (those that currently default to
hardware in the unified test helpers) to read/write that centralized API (use
names: EPreferredDevice, preferredDevice, runTest, and the test helper fixture
constructors) so every helper observes the same selected device for the process.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 07cc3eb7-5eb6-4972-a04f-2ccb0a22a806

📥 Commits

Reviewing files that changed from the base of the PR and between ae924de and 91b9d5e.

📒 Files selected for processing (2)

• tests/shaders/runtime_test_discovery.h
• tests/shaders/test_common.h

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[github-actions]

No actionable suggestions for changed features.

[LukeFrankio]

Addressed the review feedback in the latest commit:

• Centralized preferred device selection in ests/shaders/test_common.h so all fixture helpers share the same process-wide device preference.
• Updated unTest in ests/shaders/runtime_test_discovery.h so the hardware-preferred path still retries with WARP when E_INVALIDARG is encountered.
• Updated PR title to the requested <=50-character format.

[coderabbitai]

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (1)

tests/shaders/test_common.h (1)

1-3: ⚠️ Potential issue | 🟡 Minor

Add issue linkage in PR metadata.

Since this is a bug-fix PR, add Fixes #<id> or Closes #<id> in the PR description for traceability.

As per coding guidelines **/*: suggest adding GitHub keywords like "Fixes #123" or "Closes #123" for bug fixes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/shaders/test_common.h` around lines 1 - 3, Update the PR metadata to
include an issue linkage keyword (e.g., "Fixes #<id>" or "Closes #<id>") so the
bug-fix is traceable; specifically, add a line like "Fixes `#123`" to the PR
description (and optionally the commit message that touched
tests/shaders/test_common.h) to satisfy the repository guideline requiring
GitHub issue keywords for bug-fix PRs.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/shaders/runtime_test_discovery.h`:
- Around line 227-237: The retry is too broad: only retry to WARP when the
E_INVALIDARG comes from hardware device/fixture initialization, not any runtime
error inside runWithDevice; so narrow the try-catch to only the
device-initialization step (or add a specific initialization wrapper) that calls
the hardware setup and catches stf::HrException with Error()==E_INVALIDARG, then
perform the software retry and call ShaderTest::SetPreferredDevice accordingly;
ensure that any stf::HrException thrown later inside runWithDevice is not
swallowed—rethrow it so real hardware runtime failures are not masked.

In `@tests/shaders/test_common.h`:
- Around line 105-108: GetPreferredGPUDeviceType() currently allows an Undecided
state to be converted by ToGPUDeviceType() into Hardware which silently bypasses
WARP fallback; modify GetPreferredGPUDeviceType() to assert/fail if the stored
preferred device is Undecided (so callers like the no-arg GetFixtureDesc()
cannot implicitly get Hardware) and document that callers must call
SetPreferredDevice() (or pass an explicit device into GetFixtureDesc(device))
before attempting hardware initialization; update references to
ToGPUDeviceType(), GetFixtureDesc(), GetPreferredGPUDeviceType(), and
SetPreferredDevice() accordingly.

---

Outside diff comments:
In `@tests/shaders/test_common.h`:
- Around line 1-3: Update the PR metadata to include an issue linkage keyword
(e.g., "Fixes #<id>" or "Closes #<id>") so the bug-fix is traceable;
specifically, add a line like "Fixes `#123`" to the PR description (and optionally
the commit message that touched tests/shaders/test_common.h) to satisfy the
repository guideline requiring GitHub issue keywords for bug-fix PRs.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5f945381-c182-4d8b-9749-1797c4b0b7e2

📥 Commits

Reviewing files that changed from the base of the PR and between 91b9d5e and 253c114.

📒 Files selected for processing (2)

• tests/shaders/runtime_test_discovery.h
• tests/shaders/test_common.h

[LukeFrankio]

Applied a follow-up fix for the new major review concern in commit 5aa49df:

• The hardware->WARP retry path is now probe-scoped: fallback on E_INVALIDARG happens only while preferred device is Undecided (initial probe), and not after hardware is already marked as preferred.
• This avoids masking later hardware-path regressions while preserving first-run compatibility on systems that cannot initialize the hardware path.
• Added a fail-fast guard for undecided preferred-device lookup in GetPreferredGPUDeviceType().
• Updated unified helper macros to pass explicit hardware device type so they do not rely on undecided implicit selection.

Re-verified with
un_shader_tests: All tests passed (164 assertions in 1 test case).

Per project context, there is no tracked issue ID to add a Fixes #... footer for this change.

[coderabbitai]

🧹 Nitpick comments (1)

tests/shaders/runtime_test_discovery.h (1)

223-229: Read preferred device once before branching.

Minor cleanup: avoid multiple GetPreferredDevice() reads and make branch intent clearer.

♻️ Proposed refactor

-			if (ShaderTest::GetPreferredDevice() == ShaderTest::EPreferredDevice::Hardware) {
+			const auto preferredDevice = ShaderTest::GetPreferredDevice();
+			if (preferredDevice == ShaderTest::EPreferredDevice::Hardware) {
 				return runWithDevice(stf::GPUDevice::EDeviceType::Hardware);
 			}
 
-			if (ShaderTest::GetPreferredDevice() == ShaderTest::EPreferredDevice::Software) {
+			if (preferredDevice == ShaderTest::EPreferredDevice::Software) {
 				return runWithDevice(stf::GPUDevice::EDeviceType::Software);
 			}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/shaders/runtime_test_discovery.h` around lines 223 - 229, Read
ShaderTest::GetPreferredDevice() once into a local variable (e.g., auto pref =
ShaderTest::GetPreferredDevice()) and use that variable for the branching
instead of calling GetPreferredDevice() twice; then dispatch to
runWithDevice(...) based on pref (compare pref to
ShaderTest::EPreferredDevice::Hardware and ::Software) to make the intent
clearer and avoid duplicate calls to GetPreferredDevice() in the discovery code
that calls runWithDevice.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/shaders/runtime_test_discovery.h`:
- Around line 223-229: Read ShaderTest::GetPreferredDevice() once into a local
variable (e.g., auto pref = ShaderTest::GetPreferredDevice()) and use that
variable for the branching instead of calling GetPreferredDevice() twice; then
dispatch to runWithDevice(...) based on pref (compare pref to
ShaderTest::EPreferredDevice::Hardware and ::Software) to make the intent
clearer and avoid duplicate calls to GetPreferredDevice() in the discovery code
that calls runWithDevice.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 72b6b9d6-fcde-4f63-93b1-451a87248dc5

📥 Commits

Reviewing files that changed from the base of the PR and between 253c114 and dd83f69.

📒 Files selected for processing (3)

• tests/shaders/runtime_test_discovery.h
• tests/shaders/test_common.h
• tests/shaders/test_helpers_unified.h

[github-actions]

✅ A pre-release build is available for this PR:
Download

[doodlum]

if dx12 is not supported on the gpu, you can use WARP in software

[LukeFrankio]

if dx12 is not supported on the gpu, you can use WARP in software

That's exactly what this PR implements on E_INVALIDARG during hardware D3D12 init, it automatically retries with software WARP. The state machine just caches the result so we don't re-probe on every test. Happy to simplify if you'd prefer a different approach.

[alandtse]

Thanks. As these are just unit tests, what is the issue you ran into that you're fixing? Just it couldn't enable DX12 and failed silently? I'm not opposed, just trying to figure out how to confirm the fix.

[LukeFrankio]

Thanks. As these are just unit tests, what is the issue you ran into that you're fixing? Just it couldn't enable DX12 and failed silently? I'm not opposed, just trying to figure out how to confirm the fix.

On my machine (AMD Ryzen 7 5700U with Vega 8 iGPU), the hardware D3D12 device initialization was failing with E_INVALIDARG before any shader code even ran, causing every single discovered test to fail identically with HRESULT 0x80070057 — not because the shaders were wrong, but because the test harness couldn't initialize the device at all:

Test failed: HRESULT of 0x80070057
...
test cases: 1 | 0 passed | 1 failed
assertions: 329 | 165 passed | 164 failed

With this fix, the harness detects that specific error on first run, falls back to software WARP automatically, and all 164 shader assertions pass. The fix is confirmed by the test output — hardware fails silently, WARP takes over, tests go green.

[alandtse]

@doodlum waiting for your approval.