chore(cec): tidy up follow-ups from #2886#2887
Merged
Merged
Conversation
Three small cleanups deferred from the CEC display-power PR:
- Gate the Playwright screenshot captures behind a
`PYTEST_CAPTURE_SCREENSHOTS=1` env var. They were useful for the
one-time UX review but ran on every CI cycle for no reason: the
test-runner.yml upload-artifact step is `if: failure()`, so the
PNGs on green runs got written and immediately discarded. Default
is now off; set the env var for local UX work.
- Drop `test_display_power_success_toast_appearance`. It drove
`Alpine.store('toasts').push('success', ...)` directly because the
test container can't perform a real CEC success, so its only
novel assertion was that the existing toast store accepts a
'success' kind — already exercised by every other toast in the
app. It only existed to capture a success-state screenshot;
with the screenshot motivation gone, the test goes too.
- Declare `responses={200: None}` on RebootViewMixin /
ShutdownViewMixin so the generated OpenAPI schema matches the
empty response body those endpoints actually return. Aligns the
pattern with DisplayPowerViewMixin (PR #2886) which declared its
responses explicitly.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Follow-up cleanup to the HDMI-CEC settings work from #2886, reducing CI overhead from Playwright artifacts, removing a contrived integration test, and improving OpenAPI accuracy for reboot/shutdown endpoints.
Changes:
- Gate Playwright screenshot generation behind
PYTEST_CAPTURE_SCREENSHOTS=1instead of capturing on every integration run. - Remove the integration test that forced a “success” toast via Alpine (non-realistic in the container).
- Explicitly declare empty
200responses for reboot/shutdown in the generated OpenAPI schema.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/test_app.py | Adds env-gated screenshot helper and removes the success-toast integration test. |
| src/anthias_server/api/views/mixins.py | Declares explicit empty-body 200 responses for reboot/shutdown endpoints in OpenAPI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`bool(os.environ.get(...))` enables the gate for any non-empty value
including '0' and 'false', which surprises anyone setting it to disable.
Switch to an explicit allowlist ('1', 'true', 'yes', 'on', case-insensitive).
Addresses Copilot review on #2887.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues Fixed
Follow-up to #2886. No new issue — these are honest cleanups I flagged after the original PR merged.
Description
Three small tidies, all narrowly scoped:
PYTEST_CAPTURE_SCREENSHOTS=1. The original PR captured PNGs for every integration test run, but CI'sUpload integration test artifactsstep is gated onif: failure()— so on green runs the files were written and immediately discarded. Default is now off; set the env var when you want them locally for UX work.test_display_power_success_toast_appearance. It droveAlpine.store('toasts').push('success', ...)directly because the test container cannot perform a real CEC success. Its only assertion was that the existing toast store accepts asuccesskind — already covered by every other toast in the app. It only existed to capture a screenshot for the one-time UX review; with that motivation gone, the test goes too.responses={200: None}onRebootViewMixin/ShutdownViewMixinso the generated OpenAPI schema accurately reflects the empty body those endpoints return. Aligns with howDisplayPowerViewMixin(from feat(settings): experimental HDMI-CEC display on/off #2886) declared its responses explicitly.Test plan
pytest -m "not integration"— 863 pass.pytest -m integration tests/test_app.py -k display_power— 3 pass (down from 4 with the contrived success-toast test removed).PYTEST_CAPTURE_SCREENSHOTS=1 pytest -m integration ... -k display_power— confirms the env-var path still drops PNGs intotest-artifacts/cec/.ruff check+ruff format --checkclean.🤖 Generated with Claude Code