You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Replace duplicate root command behavior suites with one five-row wiring smoke while retaining shared-policy assertions, one provider-to-output read, and one real playlist-add composition path.
Replace four map-backed credential-store fakes and the process-boundary failure stub with internal/credstoretest.
Consolidate authorizer fixtures, catalog transport inheritance, and saved-library chunk matrices.
Fold both tagged live tests into their owning client/credentials packages and update every Makefile/script reference.
Deleted duplicate tests and coverage owners
root.TestSearchCommandsAreWiredToAuthenticatedSession → wiring retained by root.TestAuthenticatedCommandsAreWired; command behavior remains in searchcmd.TestTrackSearchValidatesBeforeSession and searchcmd.TestCatalogSearchValidatesBeforeSession.
root.TestSearchCommandsRejectJSONBeforeSession → searchcmd.TestSearchFactoryDerivesNounSpecificText now pins the absent JSON flag for every noun.
root.TestLibraryCommandsAreWiredAndValidateBeforeSession → wiring retained by root.TestAuthenticatedCommandsAreWired; validation remains in librarycmd.TestBatchValidationHappensBeforeSession and librarycmd.TestAlbumReferencesDeduplicateAndRejectTrackKindsBeforeSession.
root.TestCatalogGetCommandsAreWiredAndValidateBeforeSession → wiring retained by root.TestAuthenticatedCommandsAreWired; validation remains in catalogcmd.TestCatalogGetValidatesBeforeSession and catalogcmd.TestCatalogGroupsHaveOnlySingularAliasesAndNoJSON.
root.TestCatalogTraversalCommandsAreWiredAndValidateBeforeSession → wiring retained by root.TestAuthenticatedCommandsAreWired; validation remains in catalogcmd.TestCatalogTraversalValidatesBeforeSession.
root.TestPlaylistCommandsUseRealSessionAndClientPath → read wiring remains in root.TestAuthenticatedCommandsAreWired, add-session composition is pinned by root.TestPlaylistAddUsesRealSessionWiring, and leaf output/validation remains in the playlist command/client suites.
client.TestCatalogGetInheritsBoundedTransportBehavior → client.TestCatalogEntryPointsInheritTransportBehavior plus the shared retry/bounds/cancellation client tests.
client.TestCatalogTraversalInheritsTransportBehavior → client.TestCatalogEntryPointsInheritTransportBehavior plus the shared retry/cancellation client tests.
client.TestSavedTrackMutationsChunkAtForty → client.TestSavedAlbumOperationsUseGenericLibraryChunks, now table-driven over album and track with complete GET/PUT/DELETE URI assertions.
client.TestSavedTrackOperationsChunkAtForty → client.TestSavedAlbumOperationsUseGenericLibraryChunks, including exact [40, 1] check/mutation chunks; length-mismatch early-stop remains in client.TestSavedAlbumOperationsRejectWrongKindsAndMalformedResponsesBeforeContinuing.
Test-count delta
493 → 478 (-15).
LOC delta
15 files, +411 / -751 (-340 net).
Verification
go build ./...
go vet ./...
make test
golangci-lint run
GOFLAGS=-tags=keyring_nopassage,spotify_live go build ./...
GOFLAGS=-tags=keyring_nopassage,spotify_live go vet ./...
go test -tags=keyring_nopassage,spotify_live ./internal/client ./internal/credentials -run '^$' (compile only)
All pass. No live-smoke mutation target was run.
Drift handled
Found four map-backed fakes; the newer processFailStore also fit naturally and was migrated, for five hand-rolled stores removed total.
Moved the newer playlist provider-contract test into internal/client as part of fully deleting internal/livesmoke.
Kept all batch-added root policy assertions and all four playlist leaf suites unchanged.
root.TestPlaylistCommandsUseRealSessionAndClientPath is not owned by the five-row root.TestAuthenticatedCommandsAreWired smoke. The smoke runs only playlists list and stops at the missing-credential error after one store open. It never exercises the separately wired OpenAddSession, nor the production session/client/authenticated HTTP path. The cited playlist command suites use injected fake sessions, while the client suites test endpoints below the root composition boundary. Therefore a broken root OpenAddSession wiring would now pass. Cheapest restoration: keep the representative read row and add one small root-level playlists items add success using a stored token plus httptest.Server, asserting the bearer header, playlist/add paths, and success output. The other four root wiring suites are adequately decomposed: one root representative proves each command family is installed and reaches its common opener, while the owning package tests cover the family leaves and validation. TestSearchCommandsRejectJSONBeforeSession is also retained exactly by the three-noun factory table.
client.TestSavedTrackMutationsChunkAtForty and client.TestSavedTrackOperationsChunkAtForty lost track-kind wire coverage. Track and album do share the generic loops and savedItemURIs, so album coverage owns the chunk control flow; however the consolidated album test only observes spotify:album: values. No saved-library client test now asserts that spotifyref.Track becomes the spotify:track: query prefix. Additionally, the check branch records sizes but asserts only the number of calls, so its exact 40/1 split is no longer pinned. Cheapest restoration: table-drive TestSavedAlbumOperationsUseGenericLibraryChunks over album and track at 41 items, assert the complete URI sequence for GET/PUT/DELETE, and assert check sizes equal [40 1] as the mutation sizes already do.
The remaining consolidations are sufficient:
client.TestCatalogGetInheritsBoundedTransportBehavior and client.TestCatalogTraversalInheritsTransportBehavior: every catalog entry point is probed through getJSON; shared tests still pin forbidden/unauthorized classification, body redaction, malformed and oversized response rejection, retries, and cancellation.
Credential fake migration: the shared fake preserves the strictest previous semantics (ErrExists without overwrite, ErrNotFound on missing get/delete, queued and persistent failures, pre-failure call counting, overwrite observation, callbacks, and close observation). The process-boundary test only invokes Set, so replacing the all-method failure stub with SetErr does not weaken it.
Authorizer fixtures: newline input remains explicit in TestAuthorizeReadsCompleteRedirectURLFromStdinWithoutBrowser; EOF-without-newline remains explicit in TestAuthorizeStdinTreatsEmptyAndSlashRootPathsEqually and other helper callers closing the pipe.
Live tests: both retain //go:build spotify_live, use valid external test packages (client_test, credentials_test), scripts and Makefile target the new packages, and make test-live-smoke passes. Tagged compile succeeds, and both TestPlaylistDuplicateURIRemovalContract and TestExpireCredential are discoverable in their new packages.
Focused non-live packages also pass (389 tests across 11 packages). No live mutation target was run.
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
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
internal/credstoretest.Deleted duplicate tests and coverage owners
root.TestSearchCommandsAreWiredToAuthenticatedSession→ wiring retained byroot.TestAuthenticatedCommandsAreWired; command behavior remains insearchcmd.TestTrackSearchValidatesBeforeSessionandsearchcmd.TestCatalogSearchValidatesBeforeSession.root.TestSearchCommandsRejectJSONBeforeSession→searchcmd.TestSearchFactoryDerivesNounSpecificTextnow pins the absent JSON flag for every noun.root.TestLibraryCommandsAreWiredAndValidateBeforeSession→ wiring retained byroot.TestAuthenticatedCommandsAreWired; validation remains inlibrarycmd.TestBatchValidationHappensBeforeSessionandlibrarycmd.TestAlbumReferencesDeduplicateAndRejectTrackKindsBeforeSession.root.TestCatalogGetCommandsAreWiredAndValidateBeforeSession→ wiring retained byroot.TestAuthenticatedCommandsAreWired; validation remains incatalogcmd.TestCatalogGetValidatesBeforeSessionandcatalogcmd.TestCatalogGroupsHaveOnlySingularAliasesAndNoJSON.root.TestCatalogTraversalCommandsAreWiredAndValidateBeforeSession→ wiring retained byroot.TestAuthenticatedCommandsAreWired; validation remains incatalogcmd.TestCatalogTraversalValidatesBeforeSession.root.TestPlaylistCommandsUseRealSessionAndClientPath→ read wiring remains inroot.TestAuthenticatedCommandsAreWired, add-session composition is pinned byroot.TestPlaylistAddUsesRealSessionWiring, and leaf output/validation remains in the playlist command/client suites.client.TestCatalogGetInheritsBoundedTransportBehavior→client.TestCatalogEntryPointsInheritTransportBehaviorplus the shared retry/bounds/cancellation client tests.client.TestCatalogTraversalInheritsTransportBehavior→client.TestCatalogEntryPointsInheritTransportBehaviorplus the shared retry/cancellation client tests.client.TestSavedTrackMutationsChunkAtForty→client.TestSavedAlbumOperationsUseGenericLibraryChunks, now table-driven over album and track with complete GET/PUT/DELETE URI assertions.client.TestSavedTrackOperationsChunkAtForty→client.TestSavedAlbumOperationsUseGenericLibraryChunks, including exact[40, 1]check/mutation chunks; length-mismatch early-stop remains inclient.TestSavedAlbumOperationsRejectWrongKindsAndMalformedResponsesBeforeContinuing.Test-count delta
493 → 478 (
-15).LOC delta
15 files, +411 / -751 (
-340net).Verification
go build ./...go vet ./...make testgolangci-lint runGOFLAGS=-tags=keyring_nopassage,spotify_live go build ./...GOFLAGS=-tags=keyring_nopassage,spotify_live go vet ./...go test -tags=keyring_nopassage,spotify_live ./internal/client ./internal/credentials -run '^$'(compile only)All pass. No live-smoke mutation target was run.
Drift handled
processFailStorealso fit naturally and was migrated, for five hand-rolled stores removed total.internal/clientas part of fully deletinginternal/livesmoke.Closes #39