test(transport): deterministically exercise cold-path failover

a653eb3·

computermode·2w ago·1 file·+25 added/-28 removed

TestColdPathFailoverWhenRedirectTargetUnreachable adopted replicas [dead, alive] from the cold-path redirect, then relied on doWithFailover's random start offset to try the dead node first so it would be marked failed. Each iteration had a 1/2 chance of hitting the alive replica first (which serves immediately and resets the node list via its X-Entire-Replicas header), so across 8 iterations the dead node was never marked failed in (1/2)^8 ≈ 0.39% of runs — an intermittent failure.

Pin the dead replica as the first node tried via stickyNode (the same in-package idiom used by the redirect/sticky tests) so the redirect-target-unreachable failover path runs deterministically, and drop the now-pointless 8-iteration probabilistic loop. No production change.

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com

Sessions

2cb795c79075View transcript

[?
Warn on Empty Transcript, Improve Transport TestsClaude Code·Opus 4.8·1 step](/content/gh/entireio/cli/session/b44b5d08-6113-479b-833d-2395caf63b45#timeline-2cb795c79075/index.html)

Changes

1

• internal/remotehelper/transport

• Mproxy_test.go+25/-28

892 unmodified lines

893
894
895
896
897
896
897
898
899
900
901
902
903
904
905
906
907
908
909
910
911
899
900
901
902
903
904
905
906
907
908
912
913
914
910
911
915
916
917
918
919
913
914
915
916
917
918
919
920
921
922
923
920
921
922
925
926
927
923
924
925
926
927

892 unmodified lines

}))
    defer entry.Close()

const iterations = 8
deadFailoverObserved := false
var failed []string
p := New(Config{
    Nodes: replicas.NodeConfig{
        EntryURL:    entry.URL,
        ClusterHost: "127.0.0.1",
    },
    Path:         "/et/alice/repo",
    OnNodeFailed: func(node string) { failed = append(failed, node) },
})
// Pin the dead (redirect-target) replica as the first node tried so the
// failover path is exercised deterministically. doWithFailover otherwise
// starts at a random offset and ~half the time reaches the alive replica
// first, leaving dead untried; the previous 8-iteration loop still skipped
// dead entirely in (1/2)^8 of runs — a real flake.
p.stickyNode = dead

for i := range iterations {
    var failed []string
    p := New(Config{
        Nodes: replicas.NodeConfig{
            EntryURL:    entry.URL,
            ClusterHost: "127.0.0.1",
        },
        Path:         "/et/alice/repo",
        OnNodeFailed: func(node string) { failed = append(failed, node) },
    })
    body, err := p.InfoRefs(context.Background(), "git-upload-pack")
    require.NoError(t, err, "failover must succeed")

body, err := p.InfoRefs(context.Background(), "git-upload-pack")
    require.NoErrorf(t, err, "iteration %d: failover must succeed", i)
got, err := io.ReadAll(body)
    require.NoError(t, err)
    _ = body.Close()
    assert.Equal(t, "refs from alive", string(got), "response must come from alive replica")

got, err := io.ReadAll(body)
    require.NoError(t, err)
    _ = body.Close()
    assert.Equal(t, "refs from alive", string(got), "iteration %d: response must come from alive replica", i)

if !slices.Equal(p.nodes, []string{aliveURL}) {
        t.Errorf("iteration %d: nodes after failover = %v, want [%s]", i, p.nodes, aliveURL)
    }
    if slices.Contains(failed, dead) {
        deadFailoverObserved = true
    }
}
if !slices.Equal(p.nodes, []string{aliveURL}) {
    t.Errorf("nodes after failover = %v, want [%s]", p.nodes, aliveURL)
}

if !deadFailoverObserved {
    t.Errorf("dead replica never marked failed across %d iterations — failover path may not be exercised", iterations)
}
if !slices.Contains(failed, dead) {
    t.Errorf("dead replica was not marked failed — failover path not exercised; failed=%v", failed)
}
}

Minternal/remotehelper/transport/proxy_test.go+25/-28