Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework tooling #664

Merged
merged 10 commits into from
Dec 9, 2024
Merged

Rework tooling #664

merged 10 commits into from
Dec 9, 2024

Conversation

CoMPaTech
Copy link
Member

@CoMPaTech CoMPaTech commented Dec 8, 2024

Review-note: this (again) reformats our JSONs ... we should decide on how much we want to re-work (or just accept as is)

  • Fixed unreachable LOGGER line in test_init (from pylon)
  • Ignored a few lints (see connected issue, not fixing in this PR)
  • Generating fixtures is optional (as to not regenerate and infinite-loop (again))
  • p1v4 data apparently was not regenerated (started with a fresh fixtures environment
  • we should double-check with Enable ruff formatting #663 but they might run the same circle (couldn't commit without it obviously)
  • leveraging ruff action flow from https://github.com/astral-sh/ruff-pre-commit (using the 'with-fix' approach)

We might need to add another issue for fixing JSON (i.e. maybe not fix it in this already big PR)

Summary by CodeRabbit

  • New Features

    • Introduced a new device configuration for a "heater_central_plug" in the JSON structure.
  • Bug Fixes

    • Enhanced error handling in various test cases to ensure proper exception management for connection failures.
  • Documentation

    • Updated comments and formatting for improved readability across various files.
  • Refactor

    • Improved code formatting and readability in multiple test files and JSON configurations.
    • Streamlined JSON structures by consolidating multi-line arrays into single-line formats for better readability.
  • Chores

    • Adjusted configuration settings in pyproject.toml for linting and formatting tools.

Copy link
Contributor

coderabbitai bot commented Dec 8, 2024

Warning

Rate limit exceeded

@CoMPaTech has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 12 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 6211fa1 and 40f5e17.

📒 Files selected for processing (1)
  • tests/test_init.py (14 hunks)

Walkthrough

The pull request includes significant updates to the .github/workflows/verify.yml, .pre-commit-config.yaml, and various JSON files. Key changes involve enhancing the caching mechanism and Python environment setup in the workflow file, adding a new hook in the pre-commit configuration, and reformatting JSON structures for improved readability. Additionally, several Python files have undergone minor formatting changes to improve code clarity, while maintaining existing functionality and logic.

Changes

File Change Summary
.github/workflows/verify.yml Updated CACHE_VERSION from 11 to 12, refined steps for creating/restoring Python virtual environments, added cache restoration checks in jobs, and standardized caching logic across jobs.
.pre-commit-config.yaml Removed ci section, added ruff-format hook, updated biome hook for checking specific directories with new parameters.
fixtures/p1v4/all_data.json Deleted file containing device configurations and statuses.
plugwise/__init__.py Improved formatting and readability in the Smile class, particularly in the constructor and various methods.
plugwise/common.py Enhanced readability in several methods of the SmileCommon class by reformatting parameters and logic.
plugwise/constants.py Added new constants related to measurements and device types, restructured existing constants for clarity.
plugwise/data.py Reformatted conditional statements in the SmileData class for better readability, modified method signature for _climate_data.
plugwise/helper.py Adjusted formatting of multi-line strings and dictionary assignments for clarity.
plugwise/legacy/data.py Added type hints to method signatures in the SmileLegacyData class.
plugwise/legacy/helper.py Minor formatting changes for code clarity without altering functionality.
plugwise/legacy/smile.py Improved code formatting in the SmileLegacyAPI class, particularly in method signatures and error message formatting.
plugwise/smile.py Updated constructor and method signatures in the SmileAPI class for better readability.
plugwise/util.py Enhanced string formatting and clarity in several utility functions.
pyproject.toml Removed tool.black section, updated tool.isort and tool.pylint configurations, added tool.ruff section for linting configurations.
scripts/tests_and_coverage.sh Replaced ruff command with biome for initial formatting checks, added echo statements for clarity in the testing process.
tests/data/*.json Numerous JSON files reformatted to consolidate multi-line arrays into single-line formats for improved readability without altering data.
tests/test_adam.py Reformatted assignments and method calls in the TestPlugwiseAdam class for clarity.
tests/test_anna.py Minor formatting changes to improve readability in test methods.
tests/test_generic.py Enhanced error handling and control flow in various test cases.
tests/test_init.py Reformatted route definitions for improved readability.
tests/test_legacy_anna.py Minor formatting improvements in the test_connect_legacy_anna method.
tests/test_legacy_generic.py Introduced a new test method for legacy system error handling.
tests/test_legacy_stretch.py Minor formatting adjustments in the TestPlugwiseStretch class for clarity.

Sequence Diagram(s)

sequenceDiagram
    participant Workflow
    participant Cache
    participant PythonEnv
    participant Tests

    Workflow->>Cache: Update CACHE_VERSION
    Workflow->>PythonEnv: Restore Python environment
    PythonEnv->>Cache: Check cache hit
    alt Cache hit
        PythonEnv-->>Workflow: Load environment
    else Cache miss
        PythonEnv-->>Cache: Create new cache
    end
    Workflow->>Tests: Run tests with coverage
Loading

Possibly related PRs

  • Replace prettier with biome #598: Updates the pre-commit configuration to replace Prettier with Biome, relevant to the main PR's focus on enhancing the Python environment setup and caching mechanism.
  • Solution for Plug set as type = heater_central #630: Addresses modifications related to the heater_central type, which may indirectly relate to improvements in the Plugwise system's device classifications.
  • Improve control_state processing #659: Focuses on improving the processing of control_state, which is directly related to the changes in the main PR that enhance the caching mechanism and modify job steps, particularly in how devices' states are managed and processed.

Suggested labels

enhancement, dependencies

Suggested reviewers

  • bouwew

Poem

🐇 In the code, a dance we weave,
With caches fresh, we do believe.
Hooks and tests, all in a row,
Formatting bright, our code will glow!
So hop along, let changes sing,
For in this PR, new joys we bring! 🌟


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@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: 2

🧹 Outside diff range and nitpick comments (6)
tests/data/anna/anna_elga_2.json (1)

40-40: Consider documenting valid array values.

Since this is test data, it would be helpful to document the complete set of valid values for available_schedules and preset_modes to assist other developers in creating test cases.

Consider adding a comment in the JSON or a separate documentation file listing all valid values and their purposes.

Also applies to: 48-48

tests/test_legacy_generic.py (1)

Line range hint 12-20: Consider enhancing the test docstring.

While the test implementation is solid, the docstring could be more descriptive about the specific failure scenario being tested.

-    """Test erroneous legacy stretch system."""
+    """Test error handling when connecting to a faulty legacy stretch system.
+    
+    Verifies that InvalidXMLError is raised when attempting to connect
+    to a misconfigured legacy system.
+    """
scripts/tests_and_coverage.sh (1)

38-39: Consider making the biome path configurable.

The hard-coded path ./tmp/biome might be fragile. Consider using a variable or environment variable for the biome executable path.

+BIOME_PATH="${BIOME_PATH:-./tmp/biome}"
 echo "... biome-ing (prettier) ..."
-./tmp/biome check plugwise/ tests/ --files-ignore-unknown=true --no-errors-on-unmatched --indent-width=2 --indent-style=space --write
+"$BIOME_PATH" check plugwise/ tests/ --files-ignore-unknown=true --no-errors-on-unmatched --indent-width=2 --indent-style=space --write
tests/test_generic.py (1)

Line range hint 12-67: Consider standardizing test method docstrings.

While the test implementations are solid, the docstrings could follow a more consistent format across all test methods. Consider adopting a standard template that includes:

  • Purpose of the test
  • Expected behavior
  • Type of error being tested

Example format for one of the methods:

-    """Test P1 with invalid credentials setup."""
+    """Test authentication failure handling for P1 setup.
+
+    Verifies that:
+    - InvalidAuthentication is raised for incorrect credentials
+    - Error is properly logged at debug level
+    """
pyproject.toml (2)

60-74: Consider simplifying the init-hook

The init-hook configuration is quite complex and could potentially be simplified. Consider moving the plugin path setup to a separate configuration file or environment variable.

-init-hook = """\
-    from pathlib import Path; \
-    import sys; \
-
-    from pylint.config import find_default_config_files; \
-
-    sys.path.append( \
-        str(Path(next(find_default_config_files())).parent.joinpath('pylint/plugins'))
-    ) \
-    """
+init-hook = "import os, sys; sys.path.append(os.path.join(os.path.dirname(next(pylint.config.find_default_config_files())), 'pylint/plugins'))"

Line range hint 452-457: Consider enabling ISC001

The ISC001 rule (implicitly concatenated string literals) is commented out. Consider enabling it to catch potential string concatenation issues.

-    # "ISC001", # Implicitly concatenated string literals on one line
+    "ISC001", # Implicitly concatenated string literals on one line
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 346339a and 65740b0.

📒 Files selected for processing (49)
  • .github/workflows/verify.yml (1 hunks)
  • .pre-commit-config.yaml (2 hunks)
  • fixtures/p1v4/all_data.json (0 hunks)
  • plugwise/__init__.py (10 hunks)
  • plugwise/common.py (7 hunks)
  • plugwise/constants.py (2 hunks)
  • plugwise/data.py (7 hunks)
  • plugwise/helper.py (13 hunks)
  • plugwise/legacy/data.py (1 hunks)
  • plugwise/legacy/helper.py (6 hunks)
  • plugwise/legacy/smile.py (4 hunks)
  • plugwise/smile.py (6 hunks)
  • plugwise/util.py (4 hunks)
  • pyproject.toml (5 hunks)
  • scripts/tests_and_coverage.sh (1 hunks)
  • tests/data/adam/adam_heatpump_cooling.json (21 hunks)
  • tests/data/adam/adam_jip.json (9 hunks)
  • tests/data/adam/adam_multiple_devices_per_zone.json (9 hunks)
  • tests/data/adam/adam_onoff_cooling_fake_firmware.json (3 hunks)
  • tests/data/adam/adam_plus_anna.json (2 hunks)
  • tests/data/adam/adam_plus_anna_new.json (5 hunks)
  • tests/data/adam/adam_plus_anna_new_UPDATED_DATA.json (1 hunks)
  • tests/data/adam/adam_plus_anna_new_regulation_off.json (5 hunks)
  • tests/data/adam/adam_zone_per_device.json (10 hunks)
  • tests/data/anna/anna_elga_2.json (1 hunks)
  • tests/data/anna/anna_elga_2_cooling.json (1 hunks)
  • tests/data/anna/anna_elga_2_cooling_UPDATED_DATA.json (1 hunks)
  • tests/data/anna/anna_elga_2_schedule_off.json (1 hunks)
  • tests/data/anna/anna_elga_no_cooling.json (1 hunks)
  • tests/data/anna/anna_heatpump_cooling.json (1 hunks)
  • tests/data/anna/anna_heatpump_cooling_fake_firmware.json (1 hunks)
  • tests/data/anna/anna_heatpump_heating.json (1 hunks)
  • tests/data/anna/anna_loria_cooling_active.json (2 hunks)
  • tests/data/anna/anna_loria_driessens.json (2 hunks)
  • tests/data/anna/anna_loria_heating_idle.json (2 hunks)
  • tests/data/anna/anna_v4.json (1 hunks)
  • tests/data/anna/anna_v4_dhw.json (1 hunks)
  • tests/data/anna/anna_v4_no_tag.json (1 hunks)
  • tests/data/anna/anna_without_boiler_fw441.json (1 hunks)
  • tests/data/anna/legacy_anna.json (1 hunks)
  • tests/data/anna/legacy_anna_2.json (1 hunks)
  • tests/data/stretch/stretch_v23.json (1 hunks)
  • tests/test_adam.py (4 hunks)
  • tests/test_anna.py (3 hunks)
  • tests/test_generic.py (1 hunks)
  • tests/test_init.py (13 hunks)
  • tests/test_legacy_anna.py (1 hunks)
  • tests/test_legacy_generic.py (1 hunks)
  • tests/test_legacy_stretch.py (2 hunks)
💤 Files with no reviewable changes (1)
  • fixtures/p1v4/all_data.json
✅ Files skipped from review due to trivial changes (29)
  • tests/data/adam/adam_plus_anna_new_UPDATED_DATA.json
  • tests/test_legacy_anna.py
  • tests/data/anna/anna_elga_2_cooling.json
  • tests/data/anna/anna_heatpump_cooling.json
  • tests/data/anna/anna_v4_dhw.json
  • tests/data/anna/anna_elga_2_schedule_off.json
  • tests/data/anna/anna_heatpump_cooling_fake_firmware.json
  • tests/data/anna/legacy_anna.json
  • tests/data/anna/anna_elga_2_cooling_UPDATED_DATA.json
  • tests/data/anna/anna_without_boiler_fw441.json
  • tests/data/anna/anna_elga_no_cooling.json
  • tests/data/anna/anna_v4.json
  • tests/data/adam/adam_plus_anna_new.json
  • tests/test_legacy_stretch.py
  • tests/data/anna/anna_heatpump_heating.json
  • tests/data/anna/legacy_anna_2.json
  • tests/test_anna.py
  • tests/data/anna/anna_v4_no_tag.json
  • tests/data/adam/adam_multiple_devices_per_zone.json
  • tests/data/adam/adam_onoff_cooling_fake_firmware.json
  • tests/data/adam/adam_plus_anna_new_regulation_off.json
  • tests/data/adam/adam_jip.json
  • tests/data/adam/adam_heatpump_cooling.json
  • tests/data/adam/adam_plus_anna.json
  • tests/data/adam/adam_zone_per_device.json
  • tests/test_adam.py
  • tests/data/anna/anna_loria_driessens.json
  • tests/data/anna/anna_loria_cooling_active.json
  • tests/data/anna/anna_loria_heating_idle.json
🔇 Additional comments (37)
tests/test_legacy_generic.py (1)

8-8: LGTM! Clean class definition formatting.

The class definition is well-formatted and includes appropriate pylint directive.

plugwise/legacy/data.py (1)

5-7: LGTM! Type hints enhance code clarity.

The addition of __future__.annotations and type hints improves code maintainability and IDE support without affecting runtime behavior.

.pre-commit-config.yaml (2)

13-13: LGTM! Added ruff-format for consistent code formatting.

The addition of ruff-format hook enhances code formatting consistency across the project.


100-101: LGTM! Improved biome configuration with explicit formatting rules.

The updated biome configuration with explicit indentation rules (--indent-width=2 --indent-style=space) ensures consistent formatting across the codebase.

plugwise/util.py (4)

2-4: LGTM! Enhanced type safety with future annotations.

The addition of __future__.annotations improves type hinting support.


Line range hint 45-50: LGTM! Improved string formatting.

The string formatting in check_alternative_location is now more readable and maintainable.


69-73: LGTM! Enhanced conditional readability.

The boolean conditions in in_alternative_location are now more clearly structured.


224-225: LGTM! Improved conditional logic.

The use of the walrus operator in skip_obsolete_measurements makes the code more concise while maintaining readability.

tests/data/stretch/stretch_v23.json (2)

308-308: LGTM!

The formatting change to the members array maintains the same functionality while improving readability.


Line range hint 309-332: LGTM!

The new heater_central_plug device entry follows the established pattern and contains all required fields with reasonable values.

plugwise/legacy/smile.py (3)

142-144: LGTM!

The added section header improves code organization and makes API and service-related functions more discoverable.


185-187: LGTM!

The method signature reformatting improves readability while maintaining functionality.

Also applies to: 192-194


213-215: LGTM!

The error message formatting is now more consistent with the codebase style.

plugwise/data.py (2)

81-87: LGTM!

The conditional blocks have been reformatted for improved readability while maintaining the same logic.

Also applies to: 107-111, 245-248


292-294: LGTM!

The climate mode assignment has been reformatted for better readability without changing the functionality.

plugwise/common.py (5)

87-87: LGTM! Clean ternary operator usage.

The refactoring improves readability while maintaining the same logic.


92-94: LGTM! Clean method signature formatting.

The reformatting improves readability while maintaining proper type hints.


101-103: LGTM! Clean condition formatting.

The reformatting improves readability while maintaining the same logic.


114-116: LGTM! Clean method signature formatting.

The reformatting improves readability while maintaining proper type hints and default values.


274-276: LGTM! Clean method signature formatting.

The reformatting improves readability while maintaining proper type hints and default values.

.github/workflows/verify.yml (1)

7-7: Verify cache invalidation impact.

The cache version increment will invalidate existing caches. Ensure this is intentional and that all jobs have been tested with the new cache version.

✅ Verification successful

Cache version increment is justified

The cache version increment from 11 to 12 is warranted due to significant changes in the codebase:

  • Updates to .pre-commit-config.yaml configuration, including changes to Biome linting parameters
  • Removal of CI auto-update configuration
  • These changes affect the development environment setup and validation process
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for cache hits/misses in recent workflow runs

# Get recent workflow runs and their cache usage
gh api -X GET /repos/$GITHUB_REPOSITORY/actions/runs \
  --jq '.workflow_runs[] | select(.name=="Latest commit") | {id: .id, status: .status, conclusion: .conclusion}'

Length of output: 261


Script:

#!/bin/bash
# Let's check for recent changes in the workflow file and related cache configurations
git log -n 5 --oneline -- .github/workflows/verify.yml

# Also check the current workflow file content to understand cache usage
cat .github/workflows/verify.yml

Length of output: 15866


Script:

#!/bin/bash
# Let's check for changes in dependency files that might justify cache invalidation
git diff HEAD^1 requirements_test.txt setup.py requirements_commit.txt .pre-commit-config.yaml 2>/dev/null || true

# Also check for any recent changes in Python version
git diff HEAD^1 .github/workflows/verify.yml | grep -A 5 -B 5 "DEFAULT_PYTHON"

Length of output: 1271

plugwise/constants.py (1)

171-171: LGTM! Clean constant definition with helpful comment.

The reformatting improves readability while maintaining the same functionality. The comment clearly indicates when this measurement is available.

plugwise/__init__.py (2)

129-178: LGTM! Improved readability of API initialization.

The reformatting of the _smile_api initialization improves readability by:

  • Clearly separating SmileAPI and SmileLegacyAPI instantiation
  • Vertically aligning parameters for better visual scanning
  • Using a conditional expression for cleaner branching

Line range hint 341-452: LGTM! Enhanced error handling readability.

The error handling has been reformatted consistently across multiple methods, improving readability while maintaining the same functionality. The changes include:

  • Breaking long error messages into multiple lines
  • Consistent formatting of error message construction
plugwise/legacy/helper.py (3)

173-173: LGTM! Simplified conditional logic.

The conditional checks have been simplified and made more readable:

  • Removed unnecessary parentheses
  • Improved case statement formatting

Also applies to: 213-221


232-234: LGTM! Improved function call readability.

The _get_module_data function call has been reformatted for better readability by:

  • Breaking parameters into multiple lines
  • Consistent indentation

458-460: LGTM! Enhanced condition check readability.

The condition check for directives has been reformatted to improve readability by:

  • Breaking the condition into multiple lines
  • Consistent indentation
plugwise/smile.py (5)

135-144: LGTM! Improved assignment readability.

The assignment to self._cooling_enabled has been reformatted for better readability by:

  • Breaking the nested dictionary access into multiple lines
  • Consistent indentation

153-155: LGTM! Added clear section separator.

Added a clear section separator comment to improve code organization and readability.


227-229: LGTM! Enhanced method signature readability.

The set_select method signature has been reformatted for better readability by:

  • Breaking parameters into multiple lines
  • Consistent indentation

261-266: LGTM! Improved time calculation readability.

The time calculation has been reformatted for better readability by:

  • Breaking the chain of operations into multiple lines
  • Consistent indentation

460-461: LGTM! Enhanced error message readability.

The error message has been reformatted for better readability by:

  • Breaking the long message into multiple lines
  • Consistent indentation
pyproject.toml (1)

50-58: Well-documented configuration changes

The added comments clearly explain the rationale behind disabling specific pylint rules, making it easier for future maintainers to understand the decisions.

tests/test_init.py (2)

629-635: Consistent multi-line expression formatting

The multi-line expression formatting is now more consistent with the project's style. The indentation and line breaks improve readability.


694-699: Consistent try-except block formatting

The try-except blocks have been reformatted consistently throughout the file, improving readability and maintainability.

Also applies to: 754-761, 822-829, 908-915, 939-941, 970-977, 1019-1026

plugwise/helper.py (2)

891-894: Consistent dictionary initialization formatting

The dictionary initializations have been reformatted consistently throughout the file, improving readability and maintainability.

Also applies to: 1036-1040, 1042-1046, 1063-1067, 1069-1073


815-817: Consistent multi-line expression formatting

The multi-line expressions have been reformatted consistently throughout the file, improving readability.

Also applies to: 833-835, 839-841, 934-936

tests/data/anna/anna_elga_2.json Show resolved Hide resolved
scripts/tests_and_coverage.sh Outdated Show resolved Hide resolved
@CoMPaTech
Copy link
Member Author

Note: pre-commit.ci disabled for this repo as well

Copy link

codecov bot commented Dec 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (346339a) to head (40f5e17).
Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #664   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           20        20           
  Lines         3436      3439    +3     
=========================================
+ Hits          3436      3439    +3     

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

Copy link

sonarqubecloud bot commented Dec 8, 2024

Copy link
Contributor

@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: 0

🧹 Outside diff range and nitpick comments (2)
scripts/tests_and_coverage.sh (1)

31-34: Consider adding error message customization.

While the biome_format function works well, it could be more flexible by allowing custom error messages.

 biome_format() {
     ./tmp/biome check fixtures/ plugwise/ tests/ --files-ignore-unknown=true --no-errors-on-unmatched --indent-width=2 --indent-style=space --write
-    handle_command_error "biome formatting"
+    handle_command_error "${1:-biome formatting}"
 }
.pre-commit-config.yaml (1)

108-109: Consider using environment variables for biome configuration.

The biome configuration could benefit from using environment variables for formatting settings to maintain consistency across different tools.

-        entry: ./tmp/biome check fixtures/ plugwise/ tests/ --files-ignore-unknown=true --no-errors-on-unmatched --json-formatter-indent-width=2 --json-formatter-indent-style=space
+        entry: ./tmp/biome check fixtures/ plugwise/ tests/ --files-ignore-unknown=true --no-errors-on-unmatched --json-formatter-indent-width=${INDENT_WIDTH:-2} --json-formatter-indent-style=${INDENT_STYLE:-space}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 65740b0 and 6211fa1.

📒 Files selected for processing (35)
  • .github/workflows/verify.yml (2 hunks)
  • .pre-commit-config.yaml (3 hunks)
  • fixtures/adam_heatpump_cooling/all_data.json (21 hunks)
  • fixtures/adam_jip/all_data.json (9 hunks)
  • fixtures/adam_multiple_devices_per_zone/all_data.json (9 hunks)
  • fixtures/adam_onoff_cooling_fake_firmware/all_data.json (3 hunks)
  • fixtures/adam_plus_anna/all_data.json (2 hunks)
  • fixtures/adam_plus_anna_new/all_data.json (5 hunks)
  • fixtures/adam_plus_anna_new_regulation_off/all_data.json (5 hunks)
  • fixtures/adam_zone_per_device/all_data.json (10 hunks)
  • fixtures/anna_elga_2/all_data.json (1 hunks)
  • fixtures/anna_elga_2_cooling/all_data.json (1 hunks)
  • fixtures/anna_elga_2_schedule_off/all_data.json (1 hunks)
  • fixtures/anna_elga_no_cooling/all_data.json (1 hunks)
  • fixtures/anna_heatpump_cooling/all_data.json (1 hunks)
  • fixtures/anna_heatpump_cooling_fake_firmware/all_data.json (1 hunks)
  • fixtures/anna_heatpump_heating/all_data.json (1 hunks)
  • fixtures/anna_loria_cooling_active/all_data.json (2 hunks)
  • fixtures/anna_loria_driessens/all_data.json (2 hunks)
  • fixtures/anna_loria_heating_idle/all_data.json (2 hunks)
  • fixtures/anna_v4/all_data.json (1 hunks)
  • fixtures/anna_v4_dhw/all_data.json (1 hunks)
  • fixtures/anna_v4_no_tag/all_data.json (1 hunks)
  • fixtures/anna_without_boiler_fw441/all_data.json (1 hunks)
  • fixtures/legacy_anna/all_data.json (1 hunks)
  • fixtures/legacy_anna_2/all_data.json (1 hunks)
  • fixtures/m_adam_cooling/all_data.json (5 hunks)
  • fixtures/m_adam_heating/all_data.json (5 hunks)
  • fixtures/m_adam_jip/all_data.json (9 hunks)
  • fixtures/m_adam_multiple_devices_per_zone/all_data.json (9 hunks)
  • fixtures/m_anna_heatpump_cooling/all_data.json (1 hunks)
  • fixtures/m_anna_heatpump_idle/all_data.json (1 hunks)
  • fixtures/stretch_v23/all_data.json (1 hunks)
  • scripts/tests_and_coverage.sh (2 hunks)
  • tests/test_init.py (14 hunks)
✅ Files skipped from review due to trivial changes (31)
  • fixtures/anna_elga_2_cooling/all_data.json
  • fixtures/adam_plus_anna_new_regulation_off/all_data.json
  • fixtures/anna_elga_2/all_data.json
  • fixtures/anna_heatpump_heating/all_data.json
  • fixtures/anna_without_boiler_fw441/all_data.json
  • fixtures/anna_v4_no_tag/all_data.json
  • fixtures/m_anna_heatpump_cooling/all_data.json
  • fixtures/m_adam_heating/all_data.json
  • fixtures/anna_elga_2_schedule_off/all_data.json
  • fixtures/anna_v4_dhw/all_data.json
  • fixtures/anna_loria_heating_idle/all_data.json
  • fixtures/anna_v4/all_data.json
  • fixtures/legacy_anna_2/all_data.json
  • fixtures/m_anna_heatpump_idle/all_data.json
  • fixtures/anna_heatpump_cooling_fake_firmware/all_data.json
  • fixtures/anna_loria_driessens/all_data.json
  • fixtures/stretch_v23/all_data.json
  • fixtures/anna_loria_cooling_active/all_data.json
  • fixtures/anna_elga_no_cooling/all_data.json
  • fixtures/legacy_anna/all_data.json
  • fixtures/adam_plus_anna/all_data.json
  • fixtures/adam_onoff_cooling_fake_firmware/all_data.json
  • fixtures/adam_zone_per_device/all_data.json
  • fixtures/adam_jip/all_data.json
  • fixtures/adam_plus_anna_new/all_data.json
  • fixtures/anna_heatpump_cooling/all_data.json
  • fixtures/adam_multiple_devices_per_zone/all_data.json
  • fixtures/m_adam_jip/all_data.json
  • fixtures/m_adam_cooling/all_data.json
  • fixtures/adam_heatpump_cooling/all_data.json
  • fixtures/m_adam_multiple_devices_per_zone/all_data.json
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/verify.yml

104-104: shellcheck reported issue in this script: SC2086:info:5:83: Double quote to prevent globbing and word splitting

(shellcheck)


104-104: shellcheck reported issue in this script: SC2086:info:6:14: Double quote to prevent globbing and word splitting

(shellcheck)


104-104: shellcheck reported issue in this script: SC2086:info:8:17: Double quote to prevent globbing and word splitting

(shellcheck)

🪛 GitHub Check: codecov/patch
tests/test_init.py

[warning] 57-57: tests/test_init.py#L57
Added line #L57 was not covered by tests

🔇 Additional comments (14)
scripts/tests_and_coverage.sh (3)

24-29: LGTM: Well-structured error handling function.

The handle_command_error function provides consistent error handling and exit behavior across the script.


47-47: LGTM: Comprehensive error handling integration.

Error handling has been consistently integrated across all critical commands.

Also applies to: 56-56, 59-59, 63-63, 67-67


74-77: 🛠️ Refactor suggestion

Consider adding error handling for manual fixtures generation.

The manual fixtures section should include error handling for the Python script execution.

    echo "... Crafting manual fixtures ..." 
-   PYTHONPATH=$(pwd) python3 scripts/manual_fixtures.py
+   PYTHONPATH=$(pwd) python3 scripts/manual_fixtures.py
+   handle_command_error "manual fixtures generation"
    echo "... (re) biome-ing (prettier) ..."
    biome_format

Likely invalid or redundant comment.

.pre-commit-config.yaml (2)

11-15: LGTM: Well-structured ruff configuration.

The addition of the ruff-format hook and clear naming improves the pre-commit workflow.


20-20: LGTM: Clear and consistent hook naming.

The addition of descriptive names to all hooks improves maintainability and clarity.

Also applies to: 23-23, 30-31, 37-37, 43-43, 53-53, 61-61

.github/workflows/verify.yml (3)

7-7: LGTM: Cache version increment.

The cache version increment ensures clean cache updates for the modified workflow.


101-101: LGTM: Updated ruff commands.

The ruff commands have been appropriately updated to use the new format command.

Also applies to: 106-106


104-104: ⚠️ Potential issue

Fix shellcheck issues in the script.

The static analysis tools identified potential globbing and word splitting issues.

-          GITHUB_REF##*/
+          "${GITHUB_REF##*/}"

Likely invalid or redundant comment.

🧰 Tools
🪛 actionlint (1.7.4)

104-104: shellcheck reported issue in this script: SC2086:info:5:83: Double quote to prevent globbing and word splitting

(shellcheck)


104-104: shellcheck reported issue in this script: SC2086:info:6:14: Double quote to prevent globbing and word splitting

(shellcheck)


104-104: shellcheck reported issue in this script: SC2086:info:8:17: Double quote to prevent globbing and word splitting

(shellcheck)

tests/test_init.py (6)

117-123: LGTM! Improved route definitions readability

The route definitions have been consolidated into single lines per route, making the code more readable and maintainable.

Also applies to: 125-129


633-635: LGTM! Improved dictionary access readability

The dictionary access has been split across multiple lines for better readability while maintaining the same functionality.

Also applies to: 637-639


698-703: LGTM! Consistent exception handling format

The exception handling blocks have been reformatted consistently across multiple methods, improving code readability while maintaining the same error handling logic.

Also applies to: 758-760, 796-798, 826-828, 912-914, 943-945, 974-976, 1023-1025


788-790: LGTM! Improved method call readability

The method call parameters have been split across multiple lines for better readability.


961-965: LGTM! Improved list definition readability

The list definition has been split across multiple lines for better readability.


55-58: Consider adding test coverage for NO_FIXTURES check

The new environment variable check for NO_FIXTURES is not covered by tests. Consider adding test cases to verify this functionality.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 57-57: tests/test_init.py#L57
Added line #L57 was not covered by tests

@CoMPaTech CoMPaTech marked this pull request as ready for review December 8, 2024 21:24
@CoMPaTech CoMPaTech requested a review from a team as a code owner December 8, 2024 21:24
@CoMPaTech
Copy link
Member Author

Added additional checks for review in checklist form at the top ^^^

Copy link
Contributor

@bouwew bouwew left a comment

Choose a reason for hiding this comment

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

LGTM! Agreed let's take it step by step.

@bouwew bouwew merged commit 978fc32 into main Dec 9, 2024
19 checks passed
@bouwew bouwew deleted the tooling branch December 9, 2024 07:30
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.

2 participants