test: add filesystem fallback tests for analytics provider (#1119)#1276
test: add filesystem fallback tests for analytics provider (#1119)#1276muddlebee merged 9 commits intoTracer-Cloud:mainfrom
Conversation
…-Cloud#1119) - Add test for _get_or_create_anonymous_id() when file write fails (OSError) - Add test for capture_install_detected_if_needed() when marker touch fails - Both tests verify fail-safe behavior with monkeypatching (no real filesystem tricks)
…-Cloud#1119)\n\nRefactor: update helper annotations per linter suggestions
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Greptile SummaryAdds two unit tests verifying that Confidence Score: 4/5Safe to merge; single P2 style finding only All findings are P2 (import order). Test logic is correct, assertions are appropriate, and monkeypatch usage is idiomatic. No files require special attention Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[test: write_text raises OSError] --> B["_get_or_create_anonymous_id()"]
B --> C{mkdir succeeds?}
C -- yes --> D{file exists?}
D -- no --> E["write_text() → OSError"]
E --> F["except OSError → return uuid4()"]
F --> G[assert: valid UUID string]
H[test: touch raises OSError] --> I["capture_install_detected_if_needed()"]
I --> J["_touch_once(_FIRST_RUN_PATH)"]
J --> K{mkdir succeeds?}
K -- yes --> L{path exists?}
L -- no --> M["path.touch() → OSError"]
M --> N["except OSError → return False"]
N --> O["capture_install_detected_if_needed → return False"]
O --> P["assert: False, no events emitted"]
Reviews (1): Last reviewed commit: "Merge upstream/main into main" | Re-trigger Greptile |
Greptile SummaryAdds two unit tests verifying that Confidence Score: 4/5Safe to merge; single P2 style finding only All findings are P2 (import order). Test logic is correct, assertions are appropriate, and monkeypatch usage is idiomatic. No files require special attention Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[test: write_text raises OSError] --> B["_get_or_create_anonymous_id()"]
B --> C{mkdir succeeds?}
C -- yes --> D{file exists?}
D -- no --> E["write_text() → OSError"]
E --> F["except OSError → return uuid4()"]
F --> G[assert: valid UUID string]
H[test: touch raises OSError] --> I["capture_install_detected_if_needed()"]
I --> J["_touch_once(_FIRST_RUN_PATH)"]
J --> K{mkdir succeeds?}
K -- yes --> L{path exists?}
L -- no --> M["path.touch() → OSError"]
M --> N["except OSError → return False"]
N --> O["capture_install_detected_if_needed → return False"]
O --> P["assert: False, no events emitted"]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| from app.analytics import install, provider | ||
| from app.analytics.events import Event | ||
|
|
||
| from typing import NoReturn |
There was a problem hiding this comment.
from typing import NoReturn is a stdlib import but is placed after the app.* project imports, violating PEP 8 / isort ordering (stdlib → third-party → local). Linters and CI isort checks will flag this.
Move it alongside import uuid and from pathlib import Path in the stdlib block.
| from typing import NoReturn | |
| from typing import NoReturn |
|
@vidhishah2209 fix CI |
|
💜 One more reason the project grows. Thanks @vidhishah2209 — your contribution just landed!
👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome. |
2 participants
Fixes #1119
Describe the changes you have made in this PR -
Adds unit tests to verify that analytics helper functions fail gracefully when filesystem operations raise
OSError. Covers two error paths:_get_or_create_anonymous_id()— returns a non-empty, UUID-like string even when the anonymous ID file write failscapture_install_detected_if_needed()— returnsFalseand emits no events when the marker file creation failsAlso adds a
uuidimport for UUID validation in tests, and updates a raise-helper annotation totyping.NoReturnper linter suggestion.Files modified:
tests/analytics/test_provider.py— 2 new tests + minor annotation refactorCode Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
The goal was to ensure the analytics layer never crashes the CLI when filesystem operations fail in production (e.g. permission errors, read-only mounts, full disk).
I used
unittest.mock.patchto simulateOSErrorduring file writes and asserted that both helper functions return gracefully with expected fallback values — a valid UUID string for the anonymous ID helper, andFalsefor the install marker helper — rather than propagating the exception. I also confirmed no analytics events are emitted on the failing path for the install marker test.The
typing.NoReturnannotation was added to the raise-helper to satisfy the linter and accurately document intent.No alternative approaches were seriously considered — mocking is the standard, lightweight way to test error paths without touching the real filesystem.
Checklist before requesting a review
Note: Please check Allow edits from maintainers if you would like us to assist in the PR.