Skip to content

prusalink: extract press_button_and_verify fixture for button tests#170332

Open
heikkih wants to merge 11 commits into
home-assistant:devfrom
heikkih:refactor/prusalink-button-tests-fixture
Open

prusalink: extract press_button_and_verify fixture for button tests#170332
heikkih wants to merge 11 commits into
home-assistant:devfrom
heikkih:refactor/prusalink-button-tests-fixture

Conversation

@heikkih
Copy link
Copy Markdown
Contributor

@heikkih heikkih commented May 11, 2026

Proposed change

Follow-up to @balloob's review comment on #170193:

It's the same as the other tests, but would be good to throw this in a fixture to clean up the tests.

All four button tests in tests/components/prusalink/test_button.py followed the same shape:

  1. Set up the integration and assert the entity is in unknown state.
  2. Patch the matching pyprusalink method, press the button via services.async_call, assert the mock was called exactly once.
  3. Patch the method with side_effect=Conflict, press again, assert HomeAssistantError surfaces.

That repetition is now extracted into a single fixture, press_button_and_verify(entity_id, method), so each test only declares the entity/method pair it exercises plus which printer state it should be in.

Copilot review follow-up

Two comments from the updated Copilot review were explicitly re-checked before finalizing:

  1. assert_awaited_once() vs call_count

    • Kept assert_awaited_once() intentionally.
    • In this test set, PrusaLink.cancel_job, pause_job, resume_job, and continue_job are coroutine functions and patch to AsyncMock.
    • Verifying the awaited path is therefore the correct assertion behavior.
  2. Removed base _fetch_data patch

    • Kept removed intentionally.
    • The old patch targeted PrusaLinkUpdateCoordinator._fetch_data, but concrete coordinators override _fetch_data, so patching the base method does not intercept the real refresh path.
    • Keeping it would preserve dead test code rather than increase determinism.

Additionally, the helper fixture now has an explicit return type annotation:

  • Callable[[str, str], Awaitable[None]]

Net effect

  • tests/components/prusalink/test_button.py: repetitive press-and-verify logic is centralized in one fixture.
  • Test intent and scenario coverage remain unchanged.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (non-breaking change which fixes an issue)
  • Code quality improvements to existing code or addition of tests

Checklist

  • python -m pytest tests/components/prusalink/test_button.py
  • python -m mypy tests/components/prusalink/test_button.py
  • ruff check tests/components/prusalink/test_button.py
  • prek run --files tests/components/prusalink/test_button.py

All four button tests followed the same shape: assert entity in
`unknown` state, patch the API method, press the button, assert the
mock was called, then patch with `side_effect=Conflict` and assert
HomeAssistantError surfaces. Extract that into a single fixture so
the tests only declare which entity/method pair they exercise.

Per balloob's follow-up suggestion on home-assistant#170193:
home-assistant#170193 (comment)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request refactors the PrusaLink button tests by extracting the repeated “press button + verify success + verify Conflict handling” logic into a reusable fixture, reducing duplication while keeping the same test coverage and scenarios.

Changes:

  • Added a press_button_and_verify fixture to centralize button-press assertions and Conflict→HomeAssistantError behavior.
  • Updated existing button tests to use the new helper and removed unused hass_client/ClientSessionGenerator usage.
  • Kept the existing “continue button unavailable when printing” test as an explicit state assertion.

Comment thread tests/components/prusalink/test_button.py Outdated
Per Copilot's review on home-assistant#170332: patching the base class
`PrusaLinkUpdateCoordinator._fetch_data` is a no-op because each
concrete coordinator overrides `_fetch_data`, so Python's MRO never
resolves to the patched base-class method. The button press calls
`coordinator.async_request_refresh()` on every coordinator (see
`button.py:111-113`), so the original patch never actually prevented
the refresh it claimed to. The refresh still succeeds silently against
the already-mocked `mock_api` GET methods, which is why the tests pass
without the patch.

Tests stay green (6/6) and coverage on `button.py` remains 100%.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 1 comment.

Comment thread tests/components/prusalink/test_button.py Outdated
Per Copilot's review on home-assistant#170332: `AsyncMock.assert_awaited_once()`
fails if the integration ever stops awaiting the async pyprusalink
method, while `mock_calls == 1` would silently pass that regression.
`patch()` auto-detects the patched method as async since 3.8, so
`mock_meth` is already an `AsyncMock` and the new assertion is the
strictly stronger check.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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.

Copilot AI review requested due to automatic review settings May 15, 2026 08:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 3 comments.

Comment thread tests/components/prusalink/test_button.py
Comment thread tests/components/prusalink/test_button.py
Comment thread tests/components/prusalink/test_button.py Outdated
Copilot AI review requested due to automatic review settings May 20, 2026 10:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 2 comments.

Comment thread tests/components/prusalink/test_button.py Outdated
Comment thread tests/components/prusalink/test_button.py
heikkih added 2 commits May 20, 2026 13:06
The continue_job button uses the entity_id with 'workshop_' prefix
instead of just 'mock_title_'.

See home-assistant#170560
Copilot AI review requested due to automatic review settings May 20, 2026 11:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 4 comments.

Comment thread tests/components/prusalink/test_button.py
Comment thread tests/components/prusalink/test_button.py
Comment thread tests/components/prusalink/test_button.py
Comment thread tests/components/prusalink/test_button.py
Copy link
Copy Markdown
Contributor Author

@heikkih heikkih left a comment

Choose a reason for hiding this comment

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

I've updated this according to #170560 now. Ready for review :)

Copilot AI review requested due to automatic review settings May 21, 2026 09:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 2 comments.

Comment thread tests/components/prusalink/test_button.py
Comment thread tests/components/prusalink/test_button.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants