Skip to content

tests: address post-merge review feedback#322

Merged
MaStr merged 1 commit intoMaStr:mainfrom
filiplajszczak:post-merge-test-cleanups
Apr 14, 2026
Merged

tests: address post-merge review feedback#322
MaStr merged 1 commit intoMaStr:mainfrom
filiplajszczak:post-merge-test-cleanups

Conversation

@filiplajszczak
Copy link
Copy Markdown
Contributor

This follows up on a couple of useful Copilot review points from already-merged test PRs.

It addresses the factory-test feedback from #320 by resetting Inverter.num_inverters per test and by loosening the unknown-type assertion so it no longer bakes in the current typo in the production error message:

It also addresses the #321 feedback on test_production_offset_initialization_default, which was still constructing a fresh Batcontrol(...) outside the patched fixture setup:

@MaStr
Copy link
Copy Markdown
Owner

MaStr commented Apr 14, 2026

Hi, thanks for following up on these points.

I was not sure, if you are pro or contra the copilot review, that was the reason I took the comments as a 2nd opinion and not mandatory.

In the future, I'll keep the copilot review as a additional opinion. In the past, it made good review points on my changes, too.

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

Follow-up test-only changes to incorporate post-merge review feedback, improving isolation and robustness of the existing test suite.

Changes:

  • Refactors test_production_offset setup to construct Batcontrol instances via a factory fixture and ensure shutdown happens for all created instances.
  • Adds an autouse fixture in inverter factory tests to reset Inverter.num_inverters per test for deterministic behavior.
  • Loosens the unknown-inverter-type assertion to avoid coupling to the current production error-message typo.

Reviewed changes

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

File Description
tests/batcontrol/test_production_offset.py Introduces a make_batcontrol fixture to ensure instances are created within patched factory context and reliably shut down.
tests/batcontrol/inverter/test_inverter_factory.py Resets global inverter instance counter per test and relaxes the error-message regex match for unknown types.

@MaStr MaStr merged commit 855f766 into MaStr:main Apr 14, 2026
14 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