test: dont try refresh token when using vcr only when regenerating#52
Conversation
![managerbot-app[bot]](https://avatars.githubusercontent.com/in/853044?s=60&v=4){kind=small}
|
|
||
| def vcr_cassette_path(cassette_name) | ||
| direct_path = File.join(VCR.configuration.cassette_library_dir, "#{cassette_name}.yml") | ||
| return direct_path if File.exist?(direct_path) |
There was a problem hiding this comment.
The vcr_cassette_path method returns the direct path regardless of whether it exists or not when the sanitized path check is reached. However, if neither the direct path nor the sanitized path exist, the method still returns the sanitized path — this is intentional for the File.exist? check in replaying_vcr, but the method name vcr_cassette_path implies it always returns a valid path. Consider renaming to possible_vcr_cassette_path or documenting that the returned path may not exist.
| def with_vcr_adapter(provider:, model:, redact_request_body: false) | ||
| cassette_name = vcr_cassette_name | ||
| match_requests_on = redact_request_body ? %i[method uri] : %i[method uri json_body] | ||
| replaying_vcr = oauth_provider?(provider) && File.exist?(vcr_cassette_path(cassette_name)) |
There was a problem hiding this comment.
replaying_vcr is determined before the VCR.use_cassette block opens. If VCR itself creates or modifies the cassette file during recording, the File.exist? check happens at the right time (before recording starts). However, there's a subtle issue: if the cassette file exists but is empty or corrupt, replaying_vcr will be true and fake credentials will be used, potentially causing confusing test failures. Consider also checking that the file is non-empty or valid.
| end | ||
|
|
||
| def oauth_provider?(provider) | ||
| %w[anthropic_oauth_messages openai_oauth_codex].include?(provider) |
There was a problem hiding this comment.
The oauth_provider? method hardcodes provider names as strings. If new OAuth providers are added in the future, developers need to remember to update this list alongside the load_provider case statement. Consider using a shared constant or deriving this list from the case statement logic to keep them in sync.
1 participant
No description provided.