Repository: bimmerconnected/bimmer_connected
Pull request #630 (2024-07-13 10:47:27)

Improve tests and add version in CLI help output
<!--
  Thanks for your contribution to bimmer_connected!
-->
## Breaking change
<!--
  If your PR contains a breaking change, please explain that change here.
  Note: Remove this section if this PR is NOT a breaking change.
-->


## Proposed change
<!--
  Describe the big picture of your changes here to communicate to the
  maintainers why we should accept this pull request. If it fixes a bug
  or resolves a feature request, be sure to link to that issue in the
  additional information section.
-->


## Type of change
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (which adds functionality to this library)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests

## Additional information
<!--
  Details are important, and help maintainers processing your PR.
  Please be sure to fill out additional details, if applicable.
-->

- This PR fixes or closes issue: fixes #
- This PR is related to issue: 

## Checklist
<!--
  Put an `x` in the boxes that apply. You can also fill these out after
  creating the PR. If you're unsure about any of them, don't hesitate to ask.
  We're here to help! This is simply a reminder of what we are going to look
  for before merging your code.
-->

- [ ] The code change is tested and works locally.
- [ ] Tests have been added to verify that the new code works.


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/pull/630?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 100.00%. Comparing base [(`ef3db68`)](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/commit/ef3db68a9dbd8a29b459d57e743b77d6242a4a76?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected) to head [(`8c0bce4`)](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/commit/8c0bce46c5362918b243a56179ae4ec4539db794?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected).
> Report is 12 commits behind head on master.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff            @@
##            master      #630   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines         1428      1408   -20     
=========================================
- Hits          1428      1408   -20     
```

| [Flag](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/pull/630/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected) | Coverage Δ | |
|---|---|---|
| [3.10](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/pull/630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected) | `100.00% <ø> (ø)` | |
| [3.11](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/pull/630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected) | `100.00% <ø> (ø)` | |
| [3.12](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/pull/630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected) | `?` | |
| [3.8](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/pull/630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected) | `100.00% <ø> (ø)` | |
| [3.9](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/pull/630/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected) | `100.00% <ø> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected#carryforward-flags-in-the-pull-request-comment) to find out more.

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/pull/630?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected).


Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index c5d827e..21c9409 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1,6 +1,6 @@
 repos:
   - repo: https://github.com/charliermarsh/ruff-pre-commit
-    rev: v0.4.5
+    rev: v0.5.1
     hooks:
       - id: ruff
         args:
@@ -24,7 +24,7 @@ repos:
         exclude_types: [csv, json]
         exclude: ^test/responses/
   - repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v1.10.0
+    rev: v1.10.1
     hooks: 
       - id: mypy
         name: mypy
diff --git a/bimmer_connected/cli.py b/bimmer_connected/cli.py
index dda1a1c..204d5a3 100644
--- a/bimmer_connected/cli.py
+++ b/bimmer_connected/cli.py
@@ -11,6 +11,7 @@ from pathlib import Path
 
 import httpx
 
+from bimmer_connected import __version__ as 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
@@ -23,7 +24,10 @@ TEXT_VIN = "Vehicle Identification Number"
 
 def main_parser() -> argparse.ArgumentParser:
     """Create the ArgumentParser with all relevant subparsers."""
-    parser = argparse.ArgumentParser(description="Connect to MyBMW/MINI API and interact with your vehicle.")
+    parser = argparse.ArgumentParser(
+        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")
     parser.add_argument(
         "--oauth-store",
@@ -333,7 +337,7 @@ def main():
 
     if args.oauth_store.exists():
         try:
-            account.set_refresh_token(**json.load(args.oauth_store.open()))
+            account.set_refresh_token(**json.loads(args.oauth_store.read_text()))
         except json.JSONDecodeError:
             pass
 


Test patch:
diff --git a/bimmer_connected/tests/common.py b/bimmer_connected/tests/common.py
index 8989065..0ea1c8b 100644
--- a/bimmer_connected/tests/common.py
+++ b/bimmer_connected/tests/common.py
@@ -87,8 +87,10 @@ class MyBMWMockRouter(respx.MockRouter):
         self.get("/eadrax-ucs/v1/presentation/oauth/config").respond(
             200, json=load_response(RESPONSE_DIR / "auth" / "oauth_config.json")
         )
-        self.post("/gcdm/oauth/authenticate").mock(side_effect=self.authenticate_sideeffect)
-        self.post("/gcdm/oauth/token").respond(200, json=load_response(RESPONSE_DIR / "auth" / "auth_token.json"))
+        self.post("/gcdm/oauth/authenticate", name="authenticate").mock(side_effect=self.authenticate_sideeffect)
+        self.post("/gcdm/oauth/token", name="token").respond(
+            200, json=load_response(RESPONSE_DIR / "auth" / "auth_token.json")
+        )
 
         # Login to china
         self.get("/eadrax-coas/v1/cop/publickey").respond(
@@ -112,7 +114,7 @@ class MyBMWMockRouter(respx.MockRouter):
     def add_vehicle_routes(self) -> None:
         """Add routes for vehicle requests."""
 
-        self.post("/eadrax-vcs/v5/vehicle-list").mock(side_effect=self.vehicles_sideeffect)
+        self.post("/eadrax-vcs/v5/vehicle-list", name="vehicles").mock(side_effect=self.vehicles_sideeffect)
         self.get("/eadrax-vcs/v5/vehicle-data/profile").mock(side_effect=self.vehicle_profile_sideeffect)
         self.get("/eadrax-vcs/v4/vehicles/state", name="state").mock(side_effect=self.vehicle_state_sideeffect)
         self.get("/eadrax-crccs/v2/vehicles").mock(side_effect=self.vehicle_charging_settings_sideeffect)
diff --git a/bimmer_connected/tests/test_cli.py b/bimmer_connected/tests/test_cli.py
index c17779a..b0c3398 100644
--- a/bimmer_connected/tests/test_cli.py
+++ b/bimmer_connected/tests/test_cli.py
@@ -2,13 +2,15 @@ import json
 import subprocess
 import sys
 from pathlib import Path
-from unittest import mock
 
+import httpx
 import pytest
+import respx
 
 import bimmer_connected.cli
+from bimmer_connected import __version__ as VERSION
 
-from . import get_fingerprint_count
+from . import RESPONSE_DIR, get_fingerprint_count, load_response
 
 ARGS_USER_PW_REGION = ["myuser", "mypassword", "rest_of_world"]
 FIXTURE_CLI_HELP = "Connect to MyBMW/MINI API and interact with your vehicle."
@@ -19,6 +21,7 @@ def test_run_entrypoint():
     result = subprocess.run(["bimmerconnected", "--help"], capture_output=True, text=True)
 
     assert FIXTURE_CLI_HELP in result.stdout
+    assert VERSION in result.stdout
     assert result.returncode == 0
 
 
@@ -27,6 +30,7 @@ def test_run_module():
     result = subprocess.run(["python", "-m", "bimmer_connected.cli", "--help"], capture_output=True, text=True)
 
     assert FIXTURE_CLI_HELP in result.stdout
+    assert VERSION in result.stdout
     assert result.returncode == 0
 
 
@@ -138,9 +142,8 @@ def test_fingerprint(capsys: pytest.CaptureFixture, cli_home_dir: Path):
     assert len(txt_files) == 0
 
 
-@pytest.mark.usefixtures("bmw_fixture")
 @pytest.mark.usefixtures("cli_home_dir")
-def test_oauth_store_credentials(cli_home_dir: Path):
+def test_oauth_store_credentials(cli_home_dir: Path, bmw_fixture: respx.Router):
     """Test storing the oauth credentials."""
 
     assert (cli_home_dir / ".bimmer_connected.json").exists() is False
@@ -148,15 +151,18 @@ def test_oauth_store_credentials(cli_home_dir: Path):
     sys.argv = ["bimmerconnected", "status", *ARGS_USER_PW_REGION]
     bimmer_connected.cli.main()
 
+    assert bmw_fixture.routes["token"].call_count == 1
+    assert bmw_fixture.routes["vehicles"].calls[0].request.headers["authorization"] == "Bearer some_token_string"
+
     assert (cli_home_dir / ".bimmer_connected.json").exists() is True
     oauth_storage = json.loads((cli_home_dir / ".bimmer_connected.json").read_text())
 
     assert set(oauth_storage.keys()) == {"access_token", "refresh_token", "gcid"}
 
 
-@pytest.mark.usefixtures("bmw_fixture")
+# @pytest.mark.usefixtures("bmw_fixture")
 @pytest.mark.usefixtures("cli_home_dir")
-def test_oauth_load_credentials(cli_home_dir: Path):
+def test_oauth_load_credentials(cli_home_dir: Path, bmw_fixture: respx.Router):
     """Test loading and storing the oauth credentials."""
 
     demo_oauth_data = {
@@ -170,20 +176,20 @@ def test_oauth_load_credentials(cli_home_dir: Path):
 
     sys.argv = ["bimmerconnected", "status", *ARGS_USER_PW_REGION]
 
-    with mock.patch("bimmer_connected.account.MyBMWAccount.set_refresh_token") as mock_listener:
-        bimmer_connected.cli.main()
+    bimmer_connected.cli.main()
 
-        assert mock_listener.call_count == 1
-        mock_listener.assert_called_once_with(**demo_oauth_data)
+    assert bmw_fixture.routes["token"].call_count == 0
+    assert bmw_fixture.routes["vehicles"].calls[0].request.headers["authorization"] == "Bearer demo_access_token"
 
     assert (cli_home_dir / ".bimmer_connected.json").exists() is True
     oauth_storage = json.loads((cli_home_dir / ".bimmer_connected.json").read_text())
 
     assert set(oauth_storage.keys()) == {"access_token", "refresh_token", "gcid"}
 
-    assert oauth_storage["refresh_token"] != demo_oauth_data["refresh_token"]
-    assert oauth_storage["access_token"] != demo_oauth_data["access_token"]
-    assert oauth_storage["gcid"] != demo_oauth_data["gcid"]
+    # no change as the old tokens are still valid
+    assert oauth_storage["refresh_token"] == demo_oauth_data["refresh_token"]
+    assert oauth_storage["access_token"] == demo_oauth_data["access_token"]
+    assert oauth_storage["gcid"] == demo_oauth_data["gcid"]
 
 
 @pytest.mark.usefixtures("bmw_fixture")
@@ -231,3 +237,65 @@ def test_oauth_store_credentials_disabled(cli_home_dir: Path):
     bimmer_connected.cli.main()
 
     assert (cli_home_dir / ".bimmer_connected.json").exists() is False
+
+
+@pytest.mark.usefixtures("cli_home_dir")
+def test_login_refresh_token(cli_home_dir: Path, bmw_fixture: respx.Router):
+    """Test logging in with refresh token."""
+
+    # set up stored tokens
+    demo_oauth_data = {
+        "access_token": "outdated_access_token",
+        "refresh_token": "demo_refresh_token",
+        "gcid": "demo_gcid",
+    }
+
+    (cli_home_dir / ".bimmer_connected.json").write_text(json.dumps(demo_oauth_data))
+    assert (cli_home_dir / ".bimmer_connected.json").exists() is True
+
+    vehicle_routes = bmw_fixture.pop("vehicles")
+    bmw_fixture.post("/eadrax-vcs/v5/vehicle-list", name="vehicles").mock(
+        side_effect=[
+            httpx.Response(401, json=load_response(RESPONSE_DIR / "auth" / "auth_error_wrong_password.json")),
+            *[vehicle_routes.side_effect for _ in range(1000)],  # type: ignore[list-item]
+        ]
+    )
+
+    sys.argv = ["bimmerconnected", "--debug", "status", *ARGS_USER_PW_REGION]
+    bimmer_connected.cli.main()
+
+    assert bmw_fixture.routes["token"].call_count == 1
+    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
+
+
+@pytest.mark.usefixtures("cli_home_dir")
+def test_login_invalid_refresh_token(cli_home_dir: Path, bmw_fixture: respx.Router):
+    """Test logging in with an invalid refresh token."""
+
+    # set up stored tokens
+    demo_oauth_data = {
+        "refresh_token": "invalid_refresh_token",
+        "gcid": "demo_gcid",
+    }
+
+    (cli_home_dir / ".bimmer_connected.json").write_text(json.dumps(demo_oauth_data))
+    assert (cli_home_dir / ".bimmer_connected.json").exists() is True
+
+    bmw_fixture.post("/gcdm/oauth/token", name="token").mock(
+        side_effect=[
+            httpx.Response(401, json=load_response(RESPONSE_DIR / "auth" / "auth_error_wrong_password.json")),
+            *[httpx.Response(200, json=load_response(RESPONSE_DIR / "auth" / "auth_token.json")) for _ in range(1000)],
+        ]
+    )
+
+    sys.argv = ["bimmerconnected", "status", *ARGS_USER_PW_REGION]
+    bimmer_connected.cli.main()
+
+    assert bmw_fixture.routes["token"].call_count == 2
+    assert bmw_fixture.routes["authenticate"].call_count == 2
+    assert bmw_fixture.routes["vehicles"].calls[0].request.headers["authorization"] == "Bearer some_token_string"
+
+    assert (cli_home_dir / ".bimmer_connected.json").exists() is True