Skip to content

test: consolidate suites and fold the livesmoke package - #49

Merged
rianjs merged 2 commits into
mainfrom
test/39-consolidation
Jul 25, 2026
Merged

test: consolidate suites and fold the livesmoke package#49
rianjs merged 2 commits into
mainfrom
test/39-consolidation

Conversation

@rianjs

@rianjs rianjs commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • 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.TestSearchCommandsRejectJSONBeforeSessionsearchcmd.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.TestCatalogGetInheritsBoundedTransportBehaviorclient.TestCatalogEntryPointsInheritTransportBehavior plus the shared retry/bounds/cancellation client tests.
  • client.TestCatalogTraversalInheritsTransportBehaviorclient.TestCatalogEntryPointsInheritTransportBehavior plus the shared retry/cancellation client tests.
  • client.TestSavedTrackMutationsChunkAtFortyclient.TestSavedAlbumOperationsUseGenericLibraryChunks, now table-driven over album and track with complete GET/PUT/DELETE URI assertions.
  • client.TestSavedTrackOperationsChunkAtFortyclient.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.

Closes #39

@rianjs

rianjs commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

TDD assessment

Verdict: insufficient

Two deleted areas lost assertions.

  1. 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.

  2. 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.

@rianjs
rianjs merged commit 9a8d1a3 into main Jul 25, 2026
9 checks passed
@rianjs
rianjs deleted the test/39-consolidation branch July 25, 2026 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test: consolidate suites and fold the livesmoke package

1 participant