test(auth): add positive and negative unit tests for auth commands - #423
Merged
Conversation
Add a test module to src/commands/auth.rs covering the public command functions that can be exercised hermetically (without network, live keychain, or DCR flows). The existing module had zero tests despite being security-critical OAuth2 code. - token(): positive path where cfg.access_token bypasses the STORAGE singleton; additional pin-test for the empty-string Some(_) branch. - status(): unauthenticated return path, with and without an org label. - list(): empty session registry (STORAGE never touched) and populated registry (storage returns None → "no token" enrichment). - logout(): idempotent-on-empty-state and the multi-org case that removes just the targeted session entry from sessions.json. Tests use PUP_CONFIG_DIR + DD_TOKEN_STORAGE=file for isolation and serialize through crate::test_support::lock_env so sync/async tests in the module share a single mutex. No production code was modified. Co-Authored-By: Claude <noreply@anthropic.com>
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.
Summary
Add a
#[cfg(test)]module tosrc/commands/auth.rscovering the public command functions that can be exercised hermetically. The file previously had zero tests despite being security-critical OAuth2 code with ~10 public functions.Coverage Added
Eight new tests, all passing:
token()cfg.access_token = Some(...)bypasses storage. Pin test: empty-stringSome("")still bypasses (guards theif let Some(_)contract).status()Okwith and without an org label.list()logout()sessions.jsonand leaves the default-org session intact.Functions Skipped (with reasons)
login()— Requires live Dynamic Client Registration HTTP calls,open::that()to launch a browser, and a callback server waiting up to 300 seconds on an ephemeral loopback port. Cannot be tested hermetically without either a mock OAuth2 server (new dependency, rejected per REVIEW.md §4) or misrepresenting real behaviour (explicitly rejected by task constraints).refresh()— Requires a real OAuth2 token-refresh HTTP call to the Datadog IdP. Same constraints aslogin().with_storage()— Private helper, not part of the public surface the task asked to cover.status()— TheSTORAGEsingleton insrc/auth/storage.rscaptures itsbase_dirat first init (line 63-68) and cannot be reset per-test. Writing real tokens into STORAGE's frozen dir from outside theauth/storagemodule would require either widening internal visibility or duplicatingFileStoragefile-format logic. Left as-is;auth/storage.rsalready has extensive round-trip coverage forFileStoragetoken I/O.login/logout/refresh/list— Gated by#[cfg(target_arch = "wasm32")]; trivially justbail!(...). Not compiled in our test build.Implementation Notes
PUP_CONFIG_DIR(for session-registry isolation) andDD_TOKEN_STORAGE=file(to force the file backend on first singleton init). All env-mutating tests acquirecrate::test_support::lock_env()so sync and async tests in this module serialize against a single tokio mutex, avoiding theENV_LOCKvslock_env()split that would otherwise let them race each other forPUP_CONFIG_DIR.TempDirhelper is duplicated fromsrc/auth/storage.rs(two occurrences total; REVIEW.md §1 extracts on the third).Test plan
cargo test --bin pup 'commands::auth::tests'— 8/8 pass, verified stable across 5 consecutive runs.cargo clippy --bin pup --all-targets -- -D warnings— clean.cargo fmt --check— clean.cargo test --bin pup— 882/882 pass. (Note:cargo testwith all targets exhibits pre-existing intermittent failures inextensions::discovery::*andclient::test_make_dd_config_with_mock_serverdue to anENV_LOCKpoisoning issue that also reproduces onmainwithout these changes; seegit stash && cargo test. Not introduced by this PR.)🤖 Generated with Claude Code