Repository navigation
test(NoTicket): enable V2 auth refresh and invalidation coverage - #542
Merged
Merged
Conversation
lorenzhs
marked this pull request as ready for review
October 7, 2026 12:28
|
fstr
approved these changes
Oct 7, 2026
Contributor
Author
|
integration tests: https://github.com/firebolt-db/firebolt-python-sdk/actions/runs/37621863361 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



The V2 auth integration tests were copied from legacy FB1 tests with permanent skips and obsolete username/password fixture references. Enable sync and async coverage for the current FB2 client-credentials flow: refresh on local expiration, recover from a rejected token, and reject invalid credentials during refresh.
Use fresh, uncached auth objects per test and restrict these cases to remote V2 connections. Verify refresh is attempted and queries succeed rather than requiring the auth server to return a different token string. Accept the SDK's authentication/authorization error types instead of an obsolete message prefix.
Add mocked sync/async regressions for refresh returning the same token and for auth-endpoint HTTP 400/401 failures.
Validation: 25 relevant auth/client tests and pre-commit passed; all six integration cases collect. Live FB2 integration execution is pending CI because no integration credentials were available locally.
Note
Low Risk
Test-only changes with no production SDK behavior modified.
Overview
Re-enables and rewrites V2 sync/async auth integration tests that were permanently skipped and still assumed legacy username/password flows. Coverage is limited to remote connections, uses uncached
ClientCredentialsso tests can mutate secrets safely, and assertsget_new_token_generatorruns on local expiry or invalid token before queries succeed again.Credential invalidation is narrowed to the FB2 client-secret path and expects
AuthenticationErrororAuthorizationErrorinstead of a fixed error message prefix. Unit tests add mirrored sync/async client cases for refresh when the auth server returns the same access token, and for refresh failures mapping auth HTTP 400 →AuthenticationErrorand 401 →AuthorizationError.Reviewed by Cursor Bugbot for commit 2c4a6b1. Bugbot is set up for automated code reviews on this repo. Configure here.