[fukalous]

What and why

Make the all-probes-fail hardware test independent of the machine running pytest. The test now exercises fixed Windows, Linux, and Intel macOS branches while every platform probe raises; the existing Apple Silicon test continues to own the arm64-specific detection path.

This prevents a real Apple GPU from leaking into the supposedly fully mocked test on Apple Silicon hosts.

Closes #396

How I tested it

• Before the change on main@0fda9553: test_detect_never_fails_when_every_probe_does fails on macOS arm64 because detect() appends the host Apple M5 Pro.
• pytest -q tests/test_hardware_context.py (39 passed)
• pytest -q (2277 passed, 3 skipped)
• ruff check sentient tests
• git diff --check

AI assistance

I used an AI coding assistant to investigate and prepare the change. I reviewed the final diff and ran the checks above locally.

Checklist

• Tests added or updated, and pytest / npm run typecheck pass locally for what I touched
• ruff check sentient tests is clean
• The desktop-engine API contract did not change, so docs/API.md does not need an update
• No keys, tokens or personal data in the code, tests or screenshots

Summary by CodeRabbit

• Tests
  • Expanded a hardware detection failure test to cover Windows, Linux, and macOS on x86_64.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 43 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: b97dec82-3321-4f04-baa5-c305aeef31a1

Commits

Reviewing files that changed from the base of the PR and between 7462c03 and a8c4800.

Files selected for processing: 1

• tests/test_hardware_context.py

No actionable comments were generated in the recent review.

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: c7acd681-f318-4566-852f-1cc59e6f9de5

📥 Commits

Reviewing files that changed from the base of the PR and between 0fda955 and 7462c03.

📒 Files selected for processing: 1

• tests/test_hardware_context.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

Walkthrough

The all-probes-fail hardware detection test now runs for Windows, Linux, and macOS. It mocks the platform and machine architecture.

Changes: Hardware test isolation

Layer / File(s)	Summary
Set platform conditions tests/test_hardware_context.py	The test is parameterized for Windows, Linux, and macOS. It mocks sys.platform and hardware.platform.machine() as x86_64.

Priority: Low

Estimated code review effort: 1 (Trivial) | ~4 minutes

Change: Other · Severity of issue fixed: Low

Merge Risk: Minimal · previous review 7462c

This change isolates the failure-path test from host hardware while retaining separate Apple Silicon coverage; no actionable merge risk remains.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the change to isolate hardware probe failure coverage in the test.
Linked Issues check		Issue #396 requires the all-probes-fail test to be independent of the host machine. The diff parameterizes the test for Windows, Linux, and macOS, sets sys.platform, and sets `hardware.platform.mach…
Out of Scope Changes check		The only change is test isolation in tests/test_hardware_context.py. The parameterized platform cases and architecture mock directly implement issue #396. No unrelated production or API changes are …

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

_{A rabbit checks each platform’s trail,
And pins the machine so tests don’t fail.
Windows, Linux, macOS in sight,
Hardware probes meet the same test light.
No Apple GPU slips through the rail.}

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

[itsskofficial]

@fukalous, sorry for the long silence on this one; it's on us. Your PRs were waiting for a maintainer to approve CI (GitHub holds it on a contributor's first PRs), and we missed it for almost two weeks. I've approved the runs now. We'll also merge main into your branch ourselves, so there's nothing to rebase on your side, and test it on a real engine today. Thanks for your patience!

[itsskofficial]

@fukalous, I've merged main into your branch so CI runs against current main: merge commit a8c4800a on top of your 7462c03b. There were no conflicts and your change is untouched. I've also approved the CI runs for it.

[itsskofficial]

Thanks @fukalous, this is a clean fix for a real isolation bug, and sorry it waited so long.

What I ran on this Windows 11 machine, at the merge commit a8c4800a:

• pytest tests/test_hardware_context.py: 39 passed. test_detect_never_fails_when_every_probe_does now runs as [win32], [linux] and [darwin], and all three pass.
• The Apple Silicon case, simulated. I don't have an Apple Silicon Mac, so I couldn't run it on real hardware. Instead I ran the test inside a wrapper that makes the host look like an arm64 Mac: sys.platform="darwin", platform.machine()="arm64", and sysctl answering "Apple M5 Pro". With main's version of the test it fails exactly as #396 describes: Left contains one more item: {'name': 'Apple M5 Pro', 'vendor': 'apple', 'vram_gb': None, 'usable': True}. With your version it passes, because the test now pins sys.platform and platform.machine() itself.
• The arm64 path is still covered by test_apple_silicon_counts_two_thirds_of_memory, as the PR says.

No review threads. CI on a8c4800a is green. No changelog entry needed for a test-only change.

[itsskofficial]

Merged, thank you @fukalous, and sorry again for the wait. #395 has one small change left; the review there has the details.