Files
CosmicClash/server/matcher/worker_test.go
T
Josh Creek 3168dd9897 fix(multiplayer): stop the matcher worker crashing on a routine no-match pass
Found building the two-player proposal integration test (next commit):
Worker.Run treated ANY RunOnce error as fatal to the whole loop,
including domain.FormFromQueue's "no compatible candidates" -- which
is not a failure, it's the completely routine and expected outcome of
a queue whose currently-waiting players don't share a verified region
yet. Two real players with no common region formed exactly this
shape, and the entire matcher process exited -- taking matchmaking
down for every OTHER player in the same playlist, not just the
incompatible pair, since cmd/matcher runs one process per playlist.
Worse: on a real supervisor restart, the same still-incompatible
candidates are still queued, so it would crash again immediately --
an actual crash loop, not a one-off.

RunOnce's own per-call contract (return an error for source failure,
bad formation, mixed playlist, an incomplete batch, a lost durable
claim) is deliberately tested and unchanged. The fix is entirely in
Run's loop: only the three genuinely static misconfiguration errors
(nil dependencies, unsupported playlist, invalid size -- true on every
future pass just as much as this one, so retrying can never help) now
stop it, via new exported sentinels (ErrWorkerNotConfigured,
ErrUnsupportedPlaylist, ErrInvalidMatcherSize) and errors.Is. Every
other RunOnce error is a single pass's worth of "no match formed this
time" and Run keeps polling.

Covered by two new tests: Run recovering from a first-pass error and
still forming a proposal once the pool becomes viable (a real
concurrent goroutine driving Run, not just calling RunOnce directly --
Run's own loop had no test coverage at all before this), and Run still
stopping immediately on a genuine configuration error. Both clean
across 3 runs with -race.
2026-09-01 14:26:19 +01:00

181 lines
6.4 KiB
Go

package matcher
import (
"context"
"errors"
"sync"
"testing"
"time"
"github.com/cosmic-clash/cosmic-clash/server/domain"
)
type creatorSpy struct {
calls int
err error
last domain.Proposal
ids map[string]string
}
func (c *creatorSpy) CreateProposal(_ context.Context, proposal domain.Proposal, ids map[string]string, _ time.Time) error {
c.calls++
c.last = proposal
c.ids = ids
return c.err
}
func candidates() []domain.Candidate {
now := time.Unix(1000, 0).UTC()
result := make([]domain.Candidate, 4)
for i := range result {
result[i] = domain.Candidate{TicketID: "ticket-" + string(rune('1'+i)), PlayerID: "player-" + string(rune('1'+i)), Playlist: domain.Casual, ProtocolVersion: 1, EnqueuedAt: now.Add(time.Duration(i) * time.Second), PredictedRTT: map[string]float64{"EU": 20}}
}
return result
}
func workerFor(source CandidateSource, creator ProposalCreator) Worker {
return Worker{Source: source, Creator: creator, Playlist: domain.Casual, Size: 4, Now: func() time.Time { return time.Unix(1000, 0).UTC() }, NextID: func() string { return "proposal-1234567890123456" }, Prepare: func(id string, playlist domain.Playlist, formation domain.MatchFormation, now time.Time) (domain.PreparedProposal, error) {
return domain.PrepareProposal(id, playlist, formation, nil, domain.RankedArena{}, now)
}}
}
func TestRunOnceDelegatesFinalClaimAndBindsTickets(t *testing.T) {
creator := &creatorSpy{}
worker := workerFor(func(context.Context, time.Time, domain.Playlist, int) ([]domain.Candidate, error) {
return candidates(), nil
}, creator)
formed, err := worker.RunOnce(context.Background())
if err != nil || !formed || creator.calls != 1 {
t.Fatalf("formed=%v err=%v calls=%d", formed, err, creator.calls)
}
if len(creator.ids) != 4 || creator.ids["player-1"] != "ticket-1" {
t.Fatalf("ticket bindings=%v", creator.ids)
}
}
func TestRunOnceFailsClosedOnSourceOrDurableClaimFailure(t *testing.T) {
creator := &creatorSpy{err: errors.New("serialization conflict")}
worker := workerFor(func(context.Context, time.Time, domain.Playlist, int) ([]domain.Candidate, error) {
return nil, errors.New("redis unavailable")
}, creator)
if _, err := worker.RunOnce(context.Background()); err == nil {
t.Fatal("source failure was swallowed")
}
worker.Source = func(context.Context, time.Time, domain.Playlist, int) ([]domain.Candidate, error) {
return candidates(), nil
}
if _, err := worker.RunOnce(context.Background()); err == nil {
t.Fatal("durable claim failure was swallowed")
}
if creator.calls != 1 {
t.Fatalf("creator calls=%d", creator.calls)
}
}
func TestRunOnceDoesNotClaimAnIncompleteBatch(t *testing.T) {
creator := &creatorSpy{}
worker := workerFor(func(context.Context, time.Time, domain.Playlist, int) ([]domain.Candidate, error) {
return candidates()[:3], nil
}, creator)
formed, err := worker.RunOnce(context.Background())
if err != nil || formed || creator.calls != 0 {
t.Fatalf("formed=%v err=%v calls=%d", formed, err, creator.calls)
}
}
func TestRunOnceRejectsMixedPlaylistAndDuplicateIdentityBatches(t *testing.T) {
creator := &creatorSpy{}
worker := workerFor(func(_ context.Context, _ time.Time, _ domain.Playlist, _ int) ([]domain.Candidate, error) {
batch := candidates()
batch[1].Playlist = domain.Ranked
return batch, nil
}, creator)
if _, err := worker.RunOnce(context.Background()); err == nil {
t.Fatal("mixed playlist was accepted")
}
worker.Source = func(_ context.Context, _ time.Time, _ domain.Playlist, _ int) ([]domain.Candidate, error) {
batch := candidates()
batch[1].PlayerID = batch[0].PlayerID
return batch, nil
}
if _, err := worker.RunOnce(context.Background()); err == nil {
t.Fatal("duplicate identity was accepted")
}
if creator.calls != 0 {
t.Fatalf("creator calls=%d", creator.calls)
}
}
// TestRunSurvivesPerPassErrorsAndKeepsRetrying reproduces a real production
// bug found via a live integration test (multiplayer-next.md 8.40): two real
// players queued with no verified common region formed exactly this
// "source succeeds, formation fails" shape, and the matcher process died
// entirely rather than waiting for a compatible batch -- silently taking
// matchmaking down for every other player behind them too, not just the
// incompatible pair. Run must survive a per-pass RunOnce error and try
// again next interval rather than returning immediately.
func TestRunSurvivesPerPassErrorsAndKeepsRetrying(t *testing.T) {
var mu sync.Mutex
attempts := 0
creatorCalls := 0
worker := workerFor(func(context.Context, time.Time, domain.Playlist, int) ([]domain.Candidate, error) {
mu.Lock()
defer mu.Unlock()
attempts++
if attempts == 1 {
// Same failure shape as domain.FormFromQueue's "no compatible
// candidates" -- RunOnce still returns a non-nil error here, only
// Run's handling of it is what this test is about.
return nil, errors.New("no common region")
}
return candidates(), nil
}, ProposalCreatorFunc(func(context.Context, domain.Proposal, map[string]string, time.Time) error {
mu.Lock()
defer mu.Unlock()
creatorCalls++
return nil
}))
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
defer cancel()
done := make(chan error, 1)
go func() { done <- worker.Run(ctx, 5*time.Millisecond) }()
deadline := time.After(1 * time.Second)
for {
mu.Lock()
calls := creatorCalls
mu.Unlock()
if calls > 0 {
break
}
select {
case err := <-done:
t.Fatalf("Run returned early on a per-pass error instead of retrying: %v", err)
case <-deadline:
t.Fatal("Run never recovered from the first pass's error")
default:
time.Sleep(time.Millisecond)
}
}
mu.Lock()
defer mu.Unlock()
if creatorCalls != 1 {
t.Fatalf("creator calls=%d, want exactly 1 once formation finally succeeded", creatorCalls)
}
}
// TestRunStopsImmediatelyOnConfigurationErrors is the other half of the
// fix: a genuinely static misconfiguration (true on every future pass, not
// just this one) must still stop the worker rather than spin forever.
func TestRunStopsImmediatelyOnConfigurationErrors(t *testing.T) {
worker := workerFor(func(context.Context, time.Time, domain.Playlist, int) ([]domain.Candidate, error) {
return candidates(), nil
}, &creatorSpy{})
worker.Playlist = domain.Playlist("invalid")
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
defer cancel()
err := worker.Run(ctx, 5*time.Millisecond)
if !errors.Is(err, ErrUnsupportedPlaylist) {
t.Fatalf("Run() error = %v, want ErrUnsupportedPlaylist", err)
}
}