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

Improve linting & tests #714

Merged
merged 7 commits into from
Dec 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 24 additions & 28 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ on:
push:
branches:
- master
- async
pull_request: ~

jobs:
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
9 changes: 1 addition & 8 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions bimmer_connected/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")
Expand Down
5 changes: 5 additions & 0 deletions bimmer_connected/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
10 changes: 5 additions & 5 deletions bimmer_connected/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 0 additions & 3 deletions bimmer_connected/tests/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 8 additions & 10 deletions bimmer_connected/tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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


Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down
10 changes: 5 additions & 5 deletions bimmer_connected/vehicle/fuel_and_battery.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions bimmer_connected/vehicle/remote_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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(
Expand Down
12 changes: 9 additions & 3 deletions bimmer_connected/vehicle/reports.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
):
Expand Down Expand Up @@ -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))

Expand Down
14 changes: 12 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
Loading