Skip to content

test: isolate settings-save tests from the shared on-device config - #3215

Merged
vpetersson merged 2 commits into
Screenly:masterfrom
vpetersson-bot:fix/settings-save-tests-config-isolation
Jul 27, 2026
Merged

test: isolate settings-save tests from the shared on-device config#3215
vpetersson merged 2 commits into
Screenly:masterfrom
vpetersson-bot:fix/settings-save-tests-config-isolation

Conversation

@vpetersson-bot

Copy link
Copy Markdown
Contributor

Problem

The settings_save tests in tests/test_template_views.py write to the real ~/.anthias/anthias.conf (via the settings_save view or a direct settings.save()) and don't restore it. That leaks their posted values onto the shared on-device config, breaking later tests that read defaults back from it.

It surfaces when the non-integration and integration suites run against the same /data volume (the sequence documented in CLAUDE.md):

  1. test_settings_save_invalid_default_streaming_duration posts a non-numeric duration; clamp_duration coerces it to 0 and the handler persists default_duration = 0.
  2. The integration test test_add_asset_via_url then fails with assert 10 == 0 — it expects a newly added asset to inherit the default duration, which is now 0 on disk.

CI is green today because it isolates the two suites into separate jobs with fresh containers, so the integration job never sees the polluted config. This only bites a developer running both suites locally against one stack.

Fix

  • Add a reusable _isolated_settings_conf fixture that redirects the settings singleton's conf_file to a per-test tmp path and reloads on teardown — generalising the redirect already used by _reset_review_cta — and apply it to the settings-save tests. The two timezone tests can then drop their manual save/restore bookkeeping.
  • Fix test_settings_save_invalid_default_streaming_duration: its docstring claimed to exercise the handler's except branch, but clamp_duration swallows the ValueError and returns 0, so that branch is never reached. Point the invalid value at the field the test is named for and assert the clamped result is actually persisted.

No production code changes.

Verification

  • Full non-integration suite: 1512 passed, and the on-disk default_duration stays 10 afterwards (was flipped to 0 before this change).
  • Full integration suite run back-to-back in the same volume: 63 passed (previously 1 failed).
  • ruff==0.15.21 (the pinned version): clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HJ3ucEkn62cbgPoisAZ5LQ

The settings-save tests in test_template_views.py posted through the
settings_save view (or called settings.save() directly) against the
real ~/.anthias/anthias.conf, and did not restore it. That leaks values
onto the shared on-device config and breaks later tests that read
defaults back from it.

Concretely, when the non-integration and integration suites run against
the same /data volume, test_settings_save_invalid_default_streaming_duration
persisted default_duration=0 (clamp_duration coerces the non-numeric
input to 0), after which the integration test test_add_asset_via_url
failed with `assert 10 == 0` — it expects a new asset to inherit the
default duration. CI does not hit this because it isolates the two
suites into separate jobs with fresh containers.

Add a reusable _isolated_settings_conf fixture that redirects the
settings singleton's conf_file to a per-test tmp path and reloads on
teardown (generalising the redirect already used by _reset_review_cta),
and apply it to the settings-save tests. This also lets the two timezone
tests drop their manual save/restore bookkeeping.

While here, fix test_settings_save_invalid_default_streaming_duration:
its docstring claimed to exercise the handler's except branch, but
clamp_duration swallows the ValueError and returns 0, so that branch is
never reached. Point the invalid value at the field the test is named
for and assert the clamped result is actually persisted.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HJ3ucEkn62cbgPoisAZ5LQ

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Django template-view test suite to prevent settings_save tests from mutating the shared on-device ~/.anthias/anthias.conf, which can leak state across test suites when they run against the same /data volume.

Changes:

  • Added a reusable pytest fixture to redirect the anthias_server.settings.settings singleton’s conf_file to a per-test temp path and restore it afterwards.
  • Updated settings_save-related tests to use the new isolation fixture and removed manual save/restore bookkeeping in timezone tests.
  • Corrected the invalid-duration test to target default_streaming_duration and assert the clamped persisted result.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_template_views.py
@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (master@7acda8c). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #3215   +/-   ##
=========================================
  Coverage          ?   89.46%           
=========================================
  Files             ?       76           
  Lines             ?     8340           
  Branches          ?      890           
=========================================
  Hits              ?     7461           
  Misses            ?      665           
  Partials          ?      214           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Move the conf_file reassignment and initial save() inside the try so a
failure there can't leave the settings singleton pointed at the temp
path for the rest of the session; the finally now always restores it.

Addresses PR review feedback.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HJ3ucEkn62cbgPoisAZ5LQ
@sonarqubecloud

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@vpetersson
vpetersson merged commit 92e0943 into Screenly:master Jul 27, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants