From cd4dd25b387a98a8eb0a3a3343fdcb9c2e900a8d Mon Sep 17 00:00:00 2001
From: Christopher Kolstad <chriswk@getunleash.io>
Date: Thu, 30 Nov 2023 11:48:29 +0100
Subject: [PATCH] Closing the client before reading error count solves race

---
 metrics_test.go    | 20 ++++----------------
 repository_test.go |  6 +++---
 2 files changed, 7 insertions(+), 19 deletions(-)

diff --git a/metrics_test.go b/metrics_test.go
index 934cf68..5008493 100644
--- a/metrics_test.go
+++ b/metrics_test.go
@@ -328,13 +328,13 @@ func TestMetrics_ShouldNotCountMetricsForParentToggles(t *testing.T) {
 		WithInstanceId(mockInstanceId),
 		WithListener(mockListener),
 	)
-
+	assert.Nil(err, "client should not return an error")
 	client.WaitForReady()
 	client.IsEnabled("child")
 
 	assert.EqualValues(client.metrics.bucket.Toggles["child"].Yes, 1)
 	assert.EqualValues(client.metrics.bucket.Toggles["parent"].Yes, 0)
-	client.Close()
+	err = client.Close()
 
 	assert.Nil(err, "client should not return an error")
 	assert.True(gock.IsDone(), "there should be no more mocks")
@@ -381,8 +381,8 @@ func TestMetrics_ShouldBackoffOn500(t *testing.T) {
 	client.IsEnabled("baz")
 
 	time.Sleep(320 * time.Millisecond)
-	assert.Equal(float64(3), client.metrics.errors)
 	err = client.Close()
+	assert.Equal(float64(3), client.metrics.errors)
 	assert.Nil(err, "Client should close without a problem")
 
 }
@@ -406,24 +406,12 @@ func TestMetrics_ErrorCountShouldDecreaseIfSuccessful(t *testing.T) {
 		Post("/client/metrics").
 		Persist().
 		Reply(200)
-	mockListener := &MockedListener{}
-	mockListener.On("OnReady").Return()
-	mockListener.On("OnRegistered", mock.AnythingOfType("ClientData")).Return()
-	mockListener.On("OnCount", "foo", false).Return()
-	mockListener.On("OnCount", "bar", false).Return()
-	mockListener.On("OnCount", "baz", false).Return()
-	mockListener.On("OnWarning", mock.MatchedBy(func(e error) bool {
-		return strings.HasSuffix(e.Error(), "http://foo.com/client/metrics return 500")
-	})).Return()
-	mockListener.On("OnError", mock.Anything).Return()
-	mockListener.On("OnSent", mock.Anything).Return()
 
 	client, err := NewClient(
 		WithUrl(mockerServer),
 		WithMetricsInterval(50*time.Millisecond),
 		WithAppName(mockAppName),
 		WithInstanceId(mockInstanceId),
-		WithListener(mockListener),
 	)
 	assert.Nil(err, "client should not return an error")
 
@@ -434,7 +422,7 @@ func TestMetrics_ErrorCountShouldDecreaseIfSuccessful(t *testing.T) {
 	time.Sleep(360 * time.Millisecond)
 	client.IsEnabled("foo")
 	time.Sleep(100 * time.Millisecond)
-	assert.Equal(float64(0), client.metrics.errors)
 	err = client.Close()
+	assert.Equal(float64(0), client.metrics.errors)
 	assert.Nil(err, "Client should close without a problem")
 }
diff --git a/repository_test.go b/repository_test.go
index 9593b52..2854730 100644
--- a/repository_test.go
+++ b/repository_test.go
@@ -129,6 +129,7 @@ func TestRepository_ParseAPIResponse(t *testing.T) {
 	assert.Equal(0, len(response.Segments))
 }
 
+
 func TestRepository_backs_off_on_http_statuses(t *testing.T) {
 	a := assert.New(t)
 	testCases := []struct {
@@ -157,12 +158,11 @@ func TestRepository_backs_off_on_http_statuses(t *testing.T) {
 		)
 		a.Nil(err)
 		time.Sleep(20 * time.Millisecond)
-		a.Equal(tc.errorCount, client.repository.errors)
 		err = client.Close()
+		a.Equal(tc.errorCount, client.repository.errors)
 		a.Nil(err)
 	}
 }
-
 func TestRepository_back_offs_are_gradually_reduced_on_success(t *testing.T) {
 	a := assert.New(t)
 	defer gock.Off()
@@ -183,7 +183,7 @@ func TestRepository_back_offs_are_gradually_reduced_on_success(t *testing.T) {
 	)
 	a.Nil(err)
 	client.WaitForReady()
-	a.Equal(float64(3), client.repository.errors) // 4 failures, and then one success, should reduce error count to 3
 	err = client.Close()
+	a.Equal(float64(3), client.repository.errors) // 4 failures, and then one success, should reduce error count to 3
 	a.Nil(err)
 }
\ No newline at end of file