Repository navigation
test: backfill handler-behavior tests for friction fixes 9, 11, 13, 16 - #10
Merged
Merged
Conversation
Earlier PRs (#4, #6, #9) shipped OpenAPI schema tests but lacked handler-level behavior tests. Code reviewers flagged the gap — a schema test catches "field documented" regressions but misses "field actually emitted" or "input actually parsed" regressions. This PR backfills behavior tests for four shipped behaviors: 1. upgradeNote / limitExceededNote copy (PR #9, friction #13) TestUpgradeNote_DoesNotMentionTrial — 2 sub-cases TestLimitExceededNote_DoesNotMentionTrial — 4 sub-cases Guards: no "14-day trial" framing, contains "Claim to keep" + "$9/mo", no instant.dev/start leakage. 2. POST /api/v1/whoami (PR #6, friction #9) TestWhoami_NoTokenReturns401 — 401 on missing bearer TestWhoami_ReturnsIdentityForAuthedRequest — 200 with uid/tid claims; plan_tier enrichment when DB hit Test app now wires /api/v1/whoami so this and future tests can hit it through the full RequireAuth middleware. 3. POST /deploy/new env_vars JSON parsing (PR #4, friction #11) TestDeployNew_EnvVarsJSON_Parsed_Into_InitEnv — valid JSON merges into deployment.EnvVars; underscore-prefixed keys silently stripped (_secret never leaks) TestDeployNew_EnvVarsInvalidJSON_Returns400 — malformed JSON returns 400 error="invalid_env_vars" (not a generic 500) Includes a multipartDeployBody helper that other deploy tests can reuse without colliding with stack_test.go's name. 4. upgrade_jwt in provisioning responses (PR #9, friction #16) TestAnonymousProvisionEmitsUpgradeJWT_OnDedup — dedup response includes raw upgrade_jwt JWT (no parsing) alongside the legacy upgrade URL; the two presentations of the same token must not drift. Skips cleanly when local test DB schema lags (env column). All 10 new test cases pass against postgres:16-alpine + redis:7-alpine. Total run time <1s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 11, 2026
mastermanas805
added a commit
that referenced
this pull request
Jun 3, 2026
…(bug-bash #10) (#235) PATCH /stacks/:slug/env did a non-atomic read-modify-write (GetStackEnvVars → merge-in-Go → UpdateStackEnvVars). Two concurrent PATCHes with disjoint keys both read the same snapshot and the second blind-overwrote the first, silently dropping a key. Add models.MergeStackEnvVars: one transaction that takes a SELECT ... FOR UPDATE row lock, merges the PATCH delta inside the tx, checks the 64KiB cap before the write, and commits. Concurrent PATCHes now serialize — the second reads the first's committed result. The handler calls it once; error mapping is preserved (TooLarge→413, NotFound→404, else 503). GetStackEnvVars/UpdateStackEnvVars are kept (other callers). Tests: - models.TestMergeStackEnvVars_ConcurrentPatchesNoLostUpdate: 16 concurrent disjoint-key merges against a real seeded row; asserts all 16 keys survive (the lost-update regression; passes under -race). - models.TestMergeStackEnvVars_Branches: sqlmock coverage of every arm (begin/select/unmarshal/too-large-rollback/update/commit/happy + present-only delete counting). 100% func coverage. - The two stack-env fault tests are repurposed: the atomic merge collapses the old fetch_failed/persist_failed split into ONE persist_failed surface (the merge's tx-internal SELECT ... FOR UPDATE and UPDATE still fault through the shared faultConn, since lib/pq's driver.Tx has no QueryerContext and falls back to the conn). A new forUpdateVanish driver covers the handler's merge-NotFound→404 arm. Co-authored-by: Claude Opus 4.8 (1M context) <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
1 participant
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
Earlier PRs shipped OpenAPI schema tests but lacked handler-level behavior tests — flagged as a coverage gap during review. A schema test catches "field documented" regressions but misses "field actually emitted" / "input actually parsed" regressions.
This PR backfills:
Tests run against `postgres:16-alpine` + `redis:7-alpine` (local docker run). All 10 cases pass; one skips cleanly if the test DB schema lags.
Going forward
Every behavior-changing PR ships with both an OpenAPI schema test (where applicable) and a handler/behavior test. This PR closes the historical gap; the discipline continues.
Test plan
🤖 Generated with Claude Code