From caece00e7f04d862c5e4705e05951863de3ed59c Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Tue, 1 Sep 2026 13:17:58 +0100 Subject: [PATCH] test(multiplayer): add real concurrent result-submission rating test Races 5 concurrent identical CompleteResultWithResult calls for the same RESULT_PENDING match against a real PostgreSQL instance -- the scenario behind 8.25's 'identical duplicates idempotent' claim, which every existing result test only exercised sequentially. All five must succeed (idempotent replay, not conflict), and the rating update must apply exactly once: asserted by computing the expected post-match rating independently via the same domain.CasualOpponents/UpdateRating functions and requiring an exact match, since a doubled application would compound the rating further away from baseline rather than merely producing 'some' change that a looser inequality check would miss. First draft asserted ranked_games == 1, which doesn't hold for a casual result (rankedIncrement is unconditionally 0 for casual by design) -- caught by actually running it, not by inspection. Verified clean across 6 runs with -race, plus the full integration and unit suites. --- server/store/postgres_integration_test.go | 110 ++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/server/store/postgres_integration_test.go b/server/store/postgres_integration_test.go index ba9eafd8..371cb4ca 100644 --- a/server/store/postgres_integration_test.go +++ b/server/store/postgres_integration_test.go @@ -611,6 +611,116 @@ func TestPostgreSQLResultCompletionAndOutboxAreAtomicAndReplayable(t *testing.T) } } +// TestPostgreSQLConcurrentIdenticalResultSubmissionAppliesRatingsExactlyOnce +// races real concurrent duplicate result submissions -- the scenario behind +// task 8.25's "identical duplicates idempotent" claim, which every other +// result test in this file (and the mocked-driver unit tests) only exercises +// sequentially. A game server can legitimately retry an unacknowledged +// result POST, and two such retries can land at PostgreSQL genuinely +// concurrently; every one of them must succeed (this is the identical-replay +// path, not a conflict), the match must complete exactly once, and -- the +// part that matters -- the rating update inside applyResultRatings must not +// run twice just because it raced. +func TestPostgreSQLConcurrentIdenticalResultSubmissionAppliesRatingsExactlyOnce(t *testing.T) { + db := openIntegrationPostgres(t) + applyIntegrationMigrations(t, db) + now := time.Now().UTC().Truncate(time.Microsecond) + ctx := context.Background() + for _, player := range []string{"result-race-winner", "result-race-loser"} { + if _, err := db.ExecContext(ctx, `INSERT INTO identities (player_id, steam_id) VALUES ($1, $1)`, player); err != nil { + t.Fatal(err) + } + if _, err := db.ExecContext(ctx, `INSERT INTO ratings (player_id, rating, deviation, volatility, ranked_games) VALUES ($1, 1500, 350, 0.06, 0)`, player); err != nil { + t.Fatal(err) + } + } + if _, err := db.ExecContext(ctx, `INSERT INTO matches (match_id, playlist, state, region, protocol_version, server_id) VALUES ('result-race-match', 'casual', 'RESULT_PENDING', 'NA', 1, 'result-race-server')`); err != nil { + t.Fatal(err) + } + if _, err := db.ExecContext(ctx, `INSERT INTO queue_tickets (ticket_id, player_id, playlist, state, client_build, protocol_version, enqueued_at, expires_at) VALUES ('result-race-ticket-w', 'result-race-winner', 'casual', 'LIVE', 'build-1', 1, $1, $2), ('result-race-ticket-l', 'result-race-loser', 'casual', 'LIVE', 'build-1', 1, $1, $2)`, now, now.Add(time.Minute)); err != nil { + t.Fatal(err) + } + if _, err := db.ExecContext(ctx, `INSERT INTO match_participants (match_id, player_id, ticket_id, slot, team) VALUES ('result-race-match', 'result-race-winner', 'result-race-ticket-w', 0, 0), ('result-race-match', 'result-race-loser', 'result-race-ticket-l', 1, 1)`); err != nil { + t.Fatal(err) + } + + result := domain.MatchResult{MatchID: "result-race-match", ServerID: "result-race-server", ResultNonce: "result-race-nonce-123456", Team0Score: 3, Team1Score: 1, IntegrityState: domain.IntegrityCertified} + digest := domain.ResultDigest(result) + receipt := domain.ResultReceipt{ResultID: "result-race-receipt", MatchID: result.MatchID, ResultNonce: result.ResultNonce, PayloadDigest: digest, IntegrityState: result.IntegrityState, ReceivedAt: now} + payload := []byte(`{"match_id":"result-race-match"}`) + + const attempts = 5 + var wg sync.WaitGroup + errs := make([]error, attempts) + wg.Add(attempts) + for i := 0; i < attempts; i++ { + go func(i int) { + defer wg.Done() + errs[i] = CompleteResultWithResult(ctx, db, receipt, result.ServerID, fmt.Sprintf("result-race-event-%d", i), payload, result, now) + }(i) + } + wg.Wait() + for i, err := range errs { + if err != nil { + t.Fatalf("identical concurrent submission %d failed: %v", i, err) + } + } + + var state string + if err := db.QueryRow(`SELECT state FROM matches WHERE match_id = 'result-race-match'`).Scan(&state); err != nil { + t.Fatal(err) + } + if state != "COMPLETED" { + t.Fatalf("match state = %s, want COMPLETED", state) + } + var winnerGames, loserGames int + var winnerRating, loserRating float64 + if err := db.QueryRow(`SELECT ranked_games, rating FROM ratings WHERE player_id = 'result-race-winner'`).Scan(&winnerGames, &winnerRating); err != nil { + t.Fatal(err) + } + if err := db.QueryRow(`SELECT ranked_games, rating FROM ratings WHERE player_id = 'result-race-loser'`).Scan(&loserGames, &loserRating); err != nil { + t.Fatal(err) + } + // Casual results never increment ranked_games by design (rankedIncrement + // is unconditionally 0 for domain.Casual in applyResultRatings) -- that's + // not what this test is verifying. What proves "applied exactly once, not + // N times under the race" is the rating VALUE: a second application would + // recompute from the already-updated current rating and compound further + // away from 1500, so an exact match against a single, independently + // computed application is the assertion that actually falsifies a double + // application (unlike an inequality check, which a doubled update would + // still satisfy). + if winnerGames != 0 || loserGames != 0 { + t.Fatalf("casual result should never touch ranked_games: winner=%d loser=%d", winnerGames, loserGames) + } + baseline := domain.Rating{Value: 1500, RD: 350, Volatility: 0.06} + winnerOpponents, err := domain.CasualOpponents([]domain.Opponent{{PlayerID: "result-race-loser", Rating: baseline, Score: 1}}) + if err != nil { + t.Fatal(err) + } + wantWinner, err := domain.UpdateRating(baseline, winnerOpponents, now) + if err != nil { + t.Fatal(err) + } + loserOpponents, err := domain.CasualOpponents([]domain.Opponent{{PlayerID: "result-race-winner", Rating: baseline, Score: 0}}) + if err != nil { + t.Fatal(err) + } + wantLoser, err := domain.UpdateRating(baseline, loserOpponents, now) + if err != nil { + t.Fatal(err) + } + if winnerRating != wantWinner.Value { + t.Fatalf("winner rating = %v, want exactly %v (a value between these would indicate a partial/compounded update)", winnerRating, wantWinner.Value) + } + if loserRating != wantLoser.Value { + t.Fatalf("loser rating = %v, want exactly %v", loserRating, wantLoser.Value) + } + if winnerRating <= loserRating { + t.Fatalf("winner rating %v should exceed loser rating %v after a certified result", winnerRating, loserRating) + } +} + func TestPostgreSQLRankedSeasonRolloverIsExactlyOnce(t *testing.T) { db := openIntegrationPostgres(t) applyIntegrationMigrations(t, db)