Skip to content

Commit

Permalink
Attempt to deflake rendezvous tests
Browse files Browse the repository at this point in the history
We see occasional failures on Github CI for this test where we haven't
gotten any agent strings back. This appears to be a race condition
between the server ack'ing the message and then saving the user agent
string. Move the ack after the save to ensure its there in the test.

Closes: #36 [via git-merge-pr]
  • Loading branch information
psanford committed Mar 13, 2021
1 parent 42e1480 commit f7b51a1
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 5 deletions.
4 changes: 2 additions & 2 deletions rendezvous/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestBasicClient(t *testing.T) {

gotAgents := ts.Agents()
expectAgents := [][]string{
[]string{version.AgentString, version.AgentVersion},
{version.AgentString, version.AgentVersion},
}

if !reflect.DeepEqual(gotAgents, expectAgents) {
Expand Down Expand Up @@ -131,7 +131,7 @@ func TestCustomUserAgent(t *testing.T) {

gotAgents := ts.Agents()
expectAgents := [][]string{
[]string{agentString, agentVersion},
{agentString, agentVersion},
}

if !reflect.DeepEqual(gotAgents, expectAgents) {
Expand Down
9 changes: 6 additions & 3 deletions rendezvous/rendezvousservertest/rendezvousservertest.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ func NewServer() *TestServer {
}

func (ts *TestServer) Agents() [][]string {
ts.mu.Lock()
defer ts.mu.Unlock()
return ts.agents
}

Expand Down Expand Up @@ -229,23 +231,24 @@ func (ts *TestServer) handleWS(w http.ResponseWriter, r *http.Request) {

switch m := msg.(type) {
case *msgs.Bind:
ackMsg(m.ID)

if sideID != "" {
ackMsg(m.ID)
errMsg(m.ID, m, fmt.Errorf("already bound"))
continue
}

if m.Side == "" {
ackMsg(m.ID)
errMsg(m.ID, m, fmt.Errorf("bind requires 'side'"))
continue
}

ts.mu.Lock()
ts.agents = append(ts.agents, m.ClientVersion)
ts.mu.Unlock()

sideID = m.Side

ackMsg(m.ID)
case *msgs.Allocate:
ackMsg(m.ID)

Expand Down

0 comments on commit f7b51a1

Please sign in to comment.