Skip to content

refactor(tests): consolidate property tests and optimize fixtures#604

Merged
firstof9 merged 2 commits into
mainfrom
refactor-tests
Jun 3, 2026
Merged

refactor(tests): consolidate property tests and optimize fixtures#604
firstof9 merged 2 commits into
mainfrom
refactor-tests

Conversation

@firstof9
Copy link
Copy Markdown
Owner

@firstof9 firstof9 commented Jun 3, 2026

This PR cleans up the test suite by:

  1. Consolidating over 40 redundant, boilerplate property tests in tests/test_properties.py into a single parameterized test (test_simple_properties).
  2. Optimizing the fixture definition structure in tests/conftest.py using a single _setup_charger helper function to handle duplicate statuses, configs, and WebSocket client registrations.

This reduces the codebase by roughly 800 lines of boilerplate while maintaining identical functional coverage. All 416 test cases still pass successfully.

Summary by CodeRabbit

  • Tests

    • Refactored test fixtures to centralize mock setup and reduce duplication.
    • Consolidated multiple individual property tests into parametrized test coverage.
    • Added edge-case tests for missing data, numeric conversion errors, and error handling.
  • Chores

    • Updated workflow configuration for pull request auto-labeling.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 3, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1eac6e48-8c85-4e94-96e4-362ee3b08453

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • ✅ Review completed - (🔄 Check again to review again)

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@firstof9 firstof9 changed the title Refactor property tests and optimize conftest fixtures refactor(tests): consolidate property tests and optimize fixtures Jun 3, 2026
@github-actions github-actions Bot added the refactor Code refactorings label Jun 3, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/autolabeler.yml:
- Line 9: The workflow currently includes the unsupported/unstable pull_request
type "edited" in the autolabeler triggers (pull_request.types and
pull_request_target.types); remove "edited" and instead use supported triggers
such as "synchronize" (or "opened" / "reopened") or implement explicit logic to
re-run labeling on title/body edits (e.g., add a separate workflow triggered by
issue_comment/workflow_dispatch or a webhook handler) so labels are reliably
re-evaluated; update the entries that reference "edited" accordingly (search for
pull_request.types and pull_request_target.types in autolabeler.yml).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6f33d8b1-3bcf-4342-a982-48ff7d99f3fd

📥 Commits

Reviewing files that changed from the base of the PR and between 0a4de8a and 55bf141.

📒 Files selected for processing (3)
  • .github/workflows/autolabeler.yml
  • tests/conftest.py
  • tests/test_properties.py

Comment thread .github/workflows/autolabeler.yml
@firstof9 firstof9 merged commit a611c4e into main Jun 3, 2026
14 checks passed
@firstof9 firstof9 deleted the refactor-tests branch June 3, 2026 01:48
@secondof9
Copy link
Copy Markdown

Code Review Summary for PR 604

Verdict: ✅ LGTM — The refactoring is clean and focused.

Critical Issues

  • None — No blocking concerns.

Warnings

  • tests/conftest.py:37–70 — The new _setup_charger() helper is a great DRY win, but be aware that it slightly blurs individual fixture diffs. If a developer needs to tweak just the status fixture, they'll now have to adjust the helper call rather than seeing a more granular diff.

Suggestions

  • tests/conftest.py — Consider adding a brief docstring that enumerates the exact parameters the helper accepts (e.g., "status_fixture: path to JSON fixture for /status endpoint"). This would make the helper self-documenting for future maintainers.

Looks Good

  • Consolidation of ~150 property tests into a single parameterized test is a strong DRY win
  • Test coverage remains identical — all 416 test cases still pass
  • Edge-case tests for unknown status codes and non-numeric values were preserved
  • Fixtures now clearly show their intended configuration (e.g., config_fixture="v4_json/config-new.json" is explicit)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Code refactorings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants