Improve linting & tests (#714)

* Add ruff rules A, C4, N, PL, RUF

* Apply ruff fules A, C4, N, PL, RUF

* Update test pipeline

* Fix caching

* Fix pytest asyncio warning

* Remove workaround for respx<0.22

* Update test pipeline paragraph

.github/workflows/test.yml

@@ -4,7 +4,6 @@ on:
   push:
     branches:
       - master
-      - async
   pull_request: ~
 
 jobs:
@@ -15,13 +14,15 @@ jobs:
         python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13"]
       fail-fast: false
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: Set up Python ${{ matrix.python-version }}
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python-version }}
-          allow-prereleases: true
           cache: pip
+          cache-dependency-path: |
+            requirements.txt
+            requirements-test.txt
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip
@@ -42,46 +43,38 @@ jobs:
   ruff:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: Set up Python
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: '3.x'
           cache: pip
+          cache-dependency-path: |
+            requirements.txt
+            requirements-test.txt
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip
           pip install -r requirements.txt -r requirements-test.txt
-      - name: Test with flake8
+      - name: Test linting
         run: |
-          ruff check --config=pyproject.toml bimmer_connected
-
-  black:
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v3
-      - name: Set up Python
-        uses: actions/setup-python@v4
-        with:
-          python-version: '3.x'
-          cache: pip
-      - name: Install dependencies
-        run: |
-          python -m pip install --upgrade pip
-          pip install -r requirements.txt -r requirements-test.txt
-      - name: Test with black
+          ruff check --config=pyproject.toml .
+      - name: Test formatting
         run: |
-          black --check bimmer_connected
+          ruff format --config=pyproject.toml .
 
   mypy:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: Set up Python
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: '3.x'
           cache: pip
+          cache-dependency-path: |
+            requirements.txt
+            requirements-test.txt
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip
@@ -93,12 +86,15 @@ jobs:
   docs:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: Set up Python
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: '3.x'
           cache: pip
+          cache-dependency-path: |
+            requirements.txt
+            requirements-docs.txt
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip

.pre-commit-config.yaml

@@ -5,14 +5,7 @@ repos:
       - id: ruff
         args:
           - --fix
-  - repo: https://github.com/psf/black
-    rev: 24.10.0
-    hooks:
-      - id: black
-        args:
-          - --safe
-          - --quiet
-        files: ^(bimmer_connected/.+)?[^/]+\.py$
+      - id: ruff-format
   - repo: https://github.com/codespell-project/codespell
     rev: v2.3.0
     hooks:

README.rst

@@ -148,7 +148,7 @@ let us know in advance.
 Code Contributions
 ==================
 Contributions are welcome! Please make sure that your code passes the checks in :code:`.github/workflows/test.yml`.
-We currently test against :code:`flake8`, :code:`pylint` and our own :code:`pytest` suite.
+We currently test with :code:`ruff`, :code:`mypy` (for both see, :code:`pyproject.toml`) and our own :code:`pytest` suite.
 And please add tests where it makes sense. The more the better.
 
 See the `contributing guidelines <https://github.com/bimmerconnected/bimmer_connected/blob/master/CONTRIBUTING.md>`_ for more details.

bimmer_connected/cli.py

@@ -12,7 +12,7 @@
 
 import httpx
 
-from bimmer_connected import __version__ as VERSION
+from bimmer_connected import __version__
 from bimmer_connected.account import MyBMWAccount
 from bimmer_connected.api.regions import get_region_from_name, valid_regions
 from bimmer_connected.const import DEFAULT_POI_NAME
@@ -26,7 +26,7 @@
 def main_parser() -> argparse.ArgumentParser:
     """Create the ArgumentParser with all relevant subparsers."""
     parser = argparse.ArgumentParser(
-        description=(f"Connect to MyBMW/MINI API and interact with your vehicle.\n\nVersion: {VERSION}"),
+        description=(f"Connect to MyBMW/MINI API and interact with your vehicle.\n\nVersion: {__version__}"),
         formatter_class=argparse.RawTextHelpFormatter,
     )
     parser.add_argument("--debug", help="Print debug logs.", action="store_true")

bimmer_connected/models.py

@@ -175,6 +175,11 @@ class ValueWithUnit(NamedTuple):
     value: Optional[Union[int, float]]
     unit: Optional[str]
 
+    @classmethod
+    def empty(cls):
+        """Return an empty/default ValueWithUnit."""
+        return cls(None, None)
+
 
 @dataclass
 class AnonymizedResponse:

bimmer_connected/tests/__init__.py

@@ -38,16 +38,16 @@
 REMOTE_SERVICE_RESPONSE_EVENTPOSITION = RESPONSE_DIR / "remote_services" / "eadrax_service_eventposition.json"
 
 
-def get_fingerprint_count(type: str) -> int:
+def get_fingerprint_count(call_type: str) -> int:
     """Return number of requests/fingerprints for a given type."""
 
-    if type == "vehicles":
+    if call_type == "vehicles":
         return len(CarBrands)
-    if type == "states":
+    if call_type == "states":
         return len(ALL_STATES)
-    if type == "profiles":
+    if call_type == "profiles":
         return len(ALL_PROFILES)
-    if type == "charging_settings":
+    if call_type == "charging_settings":
         return len(ALL_CHARGING_SETTINGS)
     return 0
 

bimmer_connected/tests/common.py

@@ -58,9 +58,6 @@
 class MyBMWMockRouter(respx.MockRouter):
     """Stateful MockRouter for MyBMW APIs."""
 
-    # See https://github.com/lundberg/respx/issues/277#issuecomment-2507693706
-    using = "httpx"
-
     def __init__(
         self,
         vehicles_to_load: Optional[List[str]] = None,

bimmer_connected/tests/test_cli.py

@@ -11,7 +11,7 @@
 import time_machine
 
 import bimmer_connected.cli
-from bimmer_connected import __version__ as VERSION
+from bimmer_connected import __version__
 
 from . import RESPONSE_DIR, get_fingerprint_count, load_response
 
@@ -21,18 +21,20 @@
 
 def test_run_entrypoint():
     """Test if the entrypoint is installed correctly."""
-    result = subprocess.run(["bimmerconnected", "--help"], capture_output=True, text=True)
+    result = subprocess.run(["bimmerconnected", "--help"], capture_output=True, text=True, check=False)
 
     assert FIXTURE_CLI_HELP in result.stdout
     assert result.returncode == 0
 
 
 def test_run_module():
     """Test if the module can be run as a python module."""
-    result = subprocess.run(["python", "-m", "bimmer_connected.cli", "--help"], capture_output=True, text=True)
+    result = subprocess.run(
+        ["python", "-m", "bimmer_connected.cli", "--help"], capture_output=True, text=True, check=False
+    )
 
     assert FIXTURE_CLI_HELP in result.stdout
-    assert VERSION in result.stdout
+    assert __version__ in result.stdout
     assert result.returncode == 0
 
 
@@ -339,9 +341,7 @@ def test_login_refresh_token(cli_home_dir: Path, bmw_fixture: respx.Router):
     bimmer_connected.cli.main()
 
     assert bmw_fixture.routes["token"].call_count == 1
-    # TODO: The following doesn't work with MyBMWMockRouter.using = "httpx"
-    # Need to wait for a respx update supporting httpx>=0.28.0 natively
-    # assert bmw_fixture.routes["vehicles"].calls[0].request.headers["authorization"] == "Bearer outdated_access_token"
+    assert bmw_fixture.routes["vehicles"].calls[0].request.headers["authorization"] == "Bearer outdated_access_token"
     assert bmw_fixture.routes["vehicles"].calls.last.request.headers["authorization"] == "Bearer some_token_string"
 
     assert (cli_home_dir / ".bimmer_connected.json").exists() is True
@@ -382,7 +382,6 @@ def test_login_invalid_refresh_token(cli_home_dir: Path, bmw_fixture: respx.Rout
 def test_captcha_set(capsys: pytest.CaptureFixture):
     """Test login for North America if captcha is given."""
 
-    ARGS_USER_PW_REGION = ["myuser", "mypassword", "north_america"]
     sys.argv = ["bimmerconnected", "status", "-j", "--captcha-token", "SOME_CAPTCHA_TOKEN", *ARGS_USER_PW_REGION]
     bimmer_connected.cli.main()
     result = capsys.readouterr()
@@ -397,8 +396,7 @@ def test_captcha_set(capsys: pytest.CaptureFixture):
 def test_captcha_unavailable(capsys: pytest.CaptureFixture):
     """Test login for North America failing if no captcha token was given."""
 
-    ARGS_USER_PW_REGION = ["myuser", "mypassword", "north_america"]
-    sys.argv = ["bimmerconnected", "status", "-j", *ARGS_USER_PW_REGION]
+    sys.argv = ["bimmerconnected", "status", "-j", "myuser", "mypassword", "rest_of_world"]
     with contextlib.suppress(SystemExit):
         bimmer_connected.cli.main()
     result = capsys.readouterr()

bimmer_connected/vehicle/fuel_and_battery.py

@@ -2,7 +2,7 @@
 
 import datetime
 import logging
-from dataclasses import dataclass
+from dataclasses import dataclass, field
 from typing import Any, Dict, Optional
 
 from bimmer_connected.const import ATTR_ATTRIBUTES, ATTR_STATE
@@ -35,16 +35,16 @@ class ChargingState(StrEnum):
 class FuelAndBattery(VehicleDataBase):
     """Provides an accessible version of `status.FuelAndBattery`."""
 
-    remaining_range_fuel: Optional[ValueWithUnit] = ValueWithUnit(None, None)
+    remaining_range_fuel: Optional[ValueWithUnit] = field(default_factory=ValueWithUnit.empty)
     """Get the remaining range of the vehicle on fuel."""
 
-    remaining_range_electric: Optional[ValueWithUnit] = ValueWithUnit(None, None)
+    remaining_range_electric: Optional[ValueWithUnit] = field(default_factory=ValueWithUnit.empty)
     """Get the remaining range of the vehicle on electricity."""
 
-    remaining_range_total: Optional[ValueWithUnit] = ValueWithUnit(None, None)
+    remaining_range_total: Optional[ValueWithUnit] = field(default_factory=ValueWithUnit.empty)
     """Get the total remaining range of the vehicle (fuel + electricity, if available)."""
 
-    remaining_fuel: Optional[ValueWithUnit] = ValueWithUnit(None, None)
+    remaining_fuel: Optional[ValueWithUnit] = field(default_factory=ValueWithUnit.empty)
     """Get the remaining fuel of the vehicle."""
 
     remaining_fuel_percent: Optional[int] = None

bimmer_connected/vehicle/remote_services.py

@@ -141,7 +141,7 @@ async def trigger_remote_service(
 
         return status
 
-    async def _get_remote_service_status(self, client: MyBMWClient, event_id: str) -> RemoteServiceStatus:
+    async def _get_remote_service_status(self, event_id: str) -> RemoteServiceStatus:
         """Return execution status of the last remote service that was triggered."""
 
         _LOGGER.debug("getting remote service status for '%s'", event_id)
@@ -159,7 +159,7 @@ async def _block_until_done(self, client: MyBMWClient, event_id: str) -> RemoteS
         fail_after = datetime.datetime.now(datetime.timezone.utc) + datetime.timedelta(seconds=_POLLING_TIMEOUT)
         while datetime.datetime.now(datetime.timezone.utc) < fail_after:
             await asyncio.sleep(_POLLING_CYCLE)
-            status = await self._get_remote_service_status(client, event_id)
+            status = await self._get_remote_service_status(event_id)
             _LOGGER.debug("current state of '%s' is: %s", event_id, status.state.value)
             if status.state == ExecutionState.ERROR:
                 raise MyBMWRemoteServiceError(

bimmer_connected/vehicle/reports.py

@@ -33,9 +33,9 @@ class ConditionBasedService:
     @classmethod
     def from_api_entry(
         cls,
-        type: str,
+        type: str,  # noqa: A002, pylint: disable=redefined-builtin
         status: str,
-        dateTime: Optional[str] = None,
+        dateTime: Optional[str] = None,  # noqa: N803
         mileage: Optional[int] = None,
         **kwargs,
     ):
@@ -87,7 +87,13 @@ class CheckControlMessage:
     state: CheckControlStatus
 
     @classmethod
-    def from_api_entry(cls, type: str, severity: str, longDescription: Optional[str] = None, **kwargs):
+    def from_api_entry(
+        cls,
+        type: str,  # noqa: A002, pylint: disable=redefined-builtin
+        severity: str,
+        longDescription: Optional[str] = None,  # noqa: N803
+        **kwargs,
+    ):
         """Parse a check control entry from the API format to `CheckControlMessage`."""
         return cls(type, longDescription, CheckControlStatus(severity))
 

pyproject.toml

@@ -7,6 +7,7 @@ profile = "black"
 
 [tool.pytest.ini_options]
 asyncio_mode = "strict"
+asyncio_default_fixture_loop_scope = "function"
 
 [tool.mypy]
 show_error_codes = true
@@ -22,16 +23,21 @@ exclude = [
 
 [tool.ruff.lint]
 select = [
+    "A",  # flake8-builtins
     "ASYNC",  # flake8-async
     "B",  # flake8-bugbear
-    "C",  # complexity
+    "C4",  # flake8-comprehensions
+    "C90",  # complexity
     "D",  # docstrings
     "E",  # pycodestyle
     "F",  # pyflakes/autoflake
     "FLY", # flynt
     "FURB", # refurb
     "I",  # isort
+    "N",  # pep8-naming
     "PGH004",  # Use specific rule codes when using noqa
+    "PL",  # pylint
+    "RUF",  # ruff
     "SIM",  # flake8-simplicity
     "UP",  # pyupgrade
     "W",  # pycodestyle
@@ -45,11 +51,15 @@ ignore = [
     "D100", # Missing docstring in public module
     "D105", # Missing docstring in magic method
     "D107", # Missing docstring in `__init__`
+    "PLR0913",  # Too many arguments in function definition
+    "PLR2004",  # Magic value used in comparison
 ]
 
 [tool.ruff.lint.per-file-ignores]
-"docs/source/conf.py" = ["D100"]
+"docs/source/conf.py" = ["A001", "D100"]
 "bimmer_connected/api/authentication.py" = ["D102", "D107"]
+"bimmer_connected/cli.py" = ["PLR0915"]
+"bimmer_connected/models.py" = ["N815"]
 
 [tool.ruff.lint.mccabe]
 max-complexity = 25