Skip to content

Commit

Permalink
stats: Fix flaky test TestTallyIntegration (#694)
Browse files Browse the repository at this point in the history
We try to verify metrics once the client receives a response, but it's
possible the server is still recording metrics. Wait for the channels to
close before verifying metrics to avoid any races.

Fixes #693.
  • Loading branch information
prashantv authored Apr 2, 2018
1 parent 1699c86 commit cc0c409
Showing 1 changed file with 13 additions and 9 deletions.
22 changes: 13 additions & 9 deletions stats/tally_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,6 @@ func TestTallyIntegration(t *testing.T) {
clientScope := tally.NewTestScope("" /* prefix */, nil /* tags */)
serverScope := tally.NewTestScope("" /* prefix */, nil /* tags */)

server := testutils.NewServer(t, testutils.NewOpts().SetStatsReporter(NewTallyReporter(serverScope)))
defer server.Close()
testutils.RegisterEcho(server, nil)

client := testutils.NewClient(t, testutils.NewOpts().SetStatsReporter(NewTallyReporter(clientScope)))
defer client.Close()

testutils.AssertEcho(t, client, server.PeerInfo().HostPort, server.ServiceName())

// Verify the tagged metrics from that call.
tests := []struct {
msg string
Expand Down Expand Up @@ -147,6 +138,19 @@ func TestTallyIntegration(t *testing.T) {
},
}

// Use a closure so that the server/client are closed before we verify metrics.
// Otherwise, we may attempt to verify metrics before they've been flushed by TChannel.
func() {
server := testutils.NewServer(t, testutils.NewOpts().SetStatsReporter(NewTallyReporter(serverScope)))
defer server.Close()
testutils.RegisterEcho(server, nil)

client := testutils.NewClient(t, testutils.NewOpts().SetStatsReporter(NewTallyReporter(clientScope)))
defer client.Close()

testutils.AssertEcho(t, client, server.PeerInfo().HostPort, server.ServiceName())
}()

for _, tt := range tests {
snapshot := tt.scope.Snapshot()
for _, counter := range tt.counters {
Expand Down

0 comments on commit cc0c409

Please sign in to comment.