Never hit real Garmin API in tests by matin · Pull Request #208 · matin/garth

matin · 2026-03-18T09:28:41Z

Summary

VCR now defaults to playback-only (record_mode="none"). Tests will never fall through to the real Garmin API, even if env vars leak from the runner environment.

Previously, VCR switched to recording mode based on whether GARTH_HOME was set, which caused intermittent CI failures when VCR couldn't match a cassette and hit the real API (returning 401).

Changes

Replace GARTH_HOME-based VCR mode with explicit GARTH_RECORD_CASSETTES=true
Autouse _clean_env fixture clears GARTH_HOME, GARTH_TOKEN, and disables telemetry for all tests
Use GARTH_RECORD_CASSETTES_HOME for authed_client token loading during recording
Remove unused logger from http.py
Document recording workflow in CLAUDE.md

Test plan

163 tests pass
CodeRabbit review clean

Closes #202

🤖 Generated with Claude Code

Summary by CodeRabbit

Documentation
- Clarified testing infrastructure: cassette playback defaults to playback-only and added steps for enabling cassette recording (environment-based recording and recording location).
Tests
- Improved fixtures to better clean environment variables and to toggle cassette recording based on a recording flag.
Chores
- Removed an unused logging setup.

VCR now defaults to playback-only (record_mode="none"). Tests will never fall through to the real API, even if env vars leak. Recording requires explicitly setting GARTH_RECORD_CASSETTES=true. - Replace GARTH_HOME-based VCR mode with GARTH_RECORD_CASSETTES - Autouse _clean_env fixture clears GARTH_HOME, GARTH_TOKEN, and disables telemetry for all tests - Use GARTH_RECORD_CASSETTES_HOME for authed_client token loading - Document recording workflow in CLAUDE.md Closes #202 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

coderabbitai · 2026-03-18T09:28:55Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a62f3c44-1296-46b9-ae47-15f5f741e364

📥 Commits

Reviewing files that changed from the base of the PR and between e2a4310 and b9689c8.

📒 Files selected for processing (1)

tests/conftest.py

Walkthrough

Renames and expands the autouse test fixture to clear GARTH_HOME and GARTH_TOKEN, makes VCR playback-only unless GARTH_RECORD_CASSETTES=true, adjusts authed_client/loading for cassette recording, removes an unused logging setup from src/garth/http.py, and documents VCR defaults and recording env vars.

Changes

Cohort / File(s)	Summary
Test infra & docs `tests/conftest.py`, `CLAUDE.md`	Renamed autouse fixture `_disable_telemetry` → `_clean_env`; fixture now sets `GARTH_TELEMETRY_ENABLED=false` and clears `GARTH_HOME` and `GARTH_TOKEN`; VCR fixture defaults to `record_mode="none"` unless `GARTH_RECORD_CASSETTES="true"`; authed_client logic updated to honor `GARTH_RECORD_CASSETTES`/`GARTH_RECORD_CASSETTES_HOME`; documentation added describing VCR playback-by-default and how to record cassettes.
Code cleanup `src/garth/http.py`	Removed unused `logging` import and logger initialization line (no functional changes).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Never hit real Garmin API in tests' directly describes the primary objective of the PR: ensuring VCR defaults to playback-only mode to prevent tests from calling the real API.
Linked Issues check	✅ Passed	The PR fully addresses all three coding objectives from issue `#202`: VCR defaults to playback-only [record_mode='none'], an explicit GARTH_RECORD_CASSETTES variable controls recording, and _clean_env fixture clears GARTH_HOME and GARTH_TOKEN.
Out of Scope Changes check	✅ Passed	All changes are scoped to the PR objectives: VCR configuration, environment cleanup fixture, token loading logic, and documentation. The removal of the unused logger from http.py is minor cleanup aligned with the focus.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix-vcr-never-hit-real-api

📝 Coding Plan

Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2026-03-18T09:29:49Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.97%. Comparing base (88d7402) to head (b9689c8).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #208      +/-   ##
==========================================
- Coverage   99.97%   99.97%   -0.01%     
==========================================
  Files          68       68              
  Lines        3556     3555       -1     
==========================================
- Hits         3555     3554       -1     
  Misses          1        1

Flag	Coverage Δ
unittests	`99.97% <100.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/conftest.py`:
- Around line 79-83: The code uses garth_home unconditionally when the
GARTH_RECORD_CASSETTES_HOME env var is set, which can alter test behavior even
when recording is disabled; change the branch so client.load(garth_home) only
runs when both garth_home is set AND recording mode is enabled, otherwise fall
back to client.configure(...). Implement the recording check by using the
existing test recording flag (preferably the project's pytest/fixture flag) or
by checking a dedicated env var (e.g., a boolean like GARTH_RECORDING_ENABLED)
and update the block around garth_home, client.load, and client.configure
accordingly.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0404dfc4-e7e2-49cd-a2a6-fd552078f88f

📥 Commits

Reviewing files that changed from the base of the PR and between 88d7402 and e2a4310.

📒 Files selected for processing (3)

CLAUDE.md
src/garth/http.py
tests/conftest.py

💤 Files with no reviewable changes (1)

src/garth/http.py

tests/conftest.py

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

coderabbitai bot reviewed Mar 18, 2026

View reviewed changes

tests/conftest.py Show resolved Hide resolved

matin and others added 2 commits March 18, 2026 02:45

Only load real tokens when recording cassettes

adfa26b

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

Add pragma no cover to recording-only branch in conftest

b9689c8

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

matin merged commit 51d821f into main Mar 18, 2026
24 of 26 checks passed

matin deleted the fix-vcr-never-hit-real-api branch March 18, 2026 09:47

coderabbitai bot mentioned this pull request Mar 18, 2026

Add /portal/sso/embed step to fix intermittent 401s #209

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Never hit real Garmin API in tests#208

Never hit real Garmin API in tests#208
matin merged 3 commits intomainfrom
fix-vcr-never-hit-real-api

matin commented Mar 18, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Mar 18, 2026 •

edited

Loading

Review failed

❌ Failed checks (1 warning)

Uh oh!

codecov bot commented Mar 18, 2026 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

matin commented Mar 18, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

codecov bot commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

matin commented Mar 18, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Mar 18, 2026 •

edited

Loading

codecov bot commented Mar 18, 2026 •

edited

Loading