From 3dc70848f368177badc918c11ac4b7229eacde65 Mon Sep 17 00:00:00 2001 From: Nickolay Kondratenko Date: Tue, 15 Apr 2025 14:50:06 +0300 Subject: [PATCH 1/6] update GH action --- .github/workflows/test.yml | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a8cf254..76ae353 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -17,23 +17,17 @@ jobs: - name: Checkout code uses: actions/checkout@v4 - - name: Set up Go + - name: Setup Go uses: actions/setup-go@v5 with: go-version: '1.24' - - - name: Cache Go modules - uses: actions/cache@v4 - with: - path: | - ~/.cache/go-build - ~/go/pkg/mod - key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} - restore-keys: | - ${{ runner.os }}-go- + cache-dependency-path: '**/go.sum' - name: Install dependencies - run: go mod tidy + run: go mod download + + - name: Build + run: go build -v ./... - name: Run tests with coverage run: go test ./... -v -race -coverprofile=coverage.out -covermode=atomic From 16d1d17f670740aaffffc411f1efff08173a4bab Mon Sep 17 00:00:00 2001 From: Nickolay Kondratenko Date: Tue, 15 Apr 2025 15:05:42 +0300 Subject: [PATCH 2/6] use switch instead of else if --- internal/client/client.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/client/client.go b/internal/client/client.go index 17bc1fa..8fecca4 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -42,14 +42,15 @@ func (c *Client) Connect() error { c.log.Sugar().Errorf("Error connecting: %v\n", err) return errors.New("failed to connect") } - if code == http.StatusOK { + switch code { + case http.StatusOK: c.log.Info("Connected successfully") c.initHeartbeat() return nil - } else if code == http.StatusConflict { + case http.StatusConflict: c.log.Warn("Connection exists at another device") return errors.New("failed to connect") - } else { + default: c.log.Error("Failed to connect") return errors.New("failed to connect") } From 17138b8fe58926b724cb2ed7fc82e8c6f491dc1e Mon Sep 17 00:00:00 2001 From: Nickolay Kondratenko Date: Tue, 15 Apr 2025 15:08:43 +0300 Subject: [PATCH 3/6] use guard clause --- internal/client/client.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/internal/client/client.go b/internal/client/client.go index 8fecca4..1cf7777 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -62,14 +62,15 @@ func (c *Client) Disconnect() error { c.log.Sugar().Errorf("Error disconnecting: %v\n", err) return errors.New("failed to disconnect") } + if code == http.StatusOK { c.log.Info("Disconnected successfully") c.stopHeartbeat() return nil - } else { - c.log.Error("Failed to disconnect") - return errors.New("failed to disconnect") } + + c.log.Error("Failed to disconnect") + return errors.New("failed to disconnect") } func (c *Client) heartbeat() { @@ -78,12 +79,13 @@ func (c *Client) heartbeat() { c.log.Sugar().Errorf("Error sending heartbeat: %v\n", err) return } + if code == http.StatusOK { c.log.Info("Heartbeat sent successfully") - } else { - c.log.Error("Failed to send heartbeat") return } + + c.log.Error("Failed to send heartbeat") } func (c *Client) initHeartbeat() { From 54c054732da266291b4f292c5182ad6edb8a2cc4 Mon Sep 17 00:00:00 2001 From: Nickolay Kondratenko Date: Tue, 15 Apr 2025 15:35:02 +0300 Subject: [PATCH 4/6] move test client to helper --- internal/client/client_test.go | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/internal/client/client_test.go b/internal/client/client_test.go index 73a693f..9572569 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -40,17 +40,11 @@ func TestConnect(t *testing.T) { for _, test := range tests { t.Run(test.Name, func(t *testing.T) { t.Parallel() - log := zap.NewNop() server := newMockServer(test.ReceivedResponse, http.StatusOK) defer server.Server.Close() - client := New(Config{ - Host: server.Server.URL, - UserID: "u1", - DeviceID: "d1", - Log: log, - }) + client := newClient(server.Server.URL) err := client.Connect() if test.IsError { @@ -97,17 +91,10 @@ func TestDisconnect(t *testing.T) { t.Run(test.Name, func(t *testing.T) { t.Parallel() - log := zap.NewNop() - server := newMockServer(http.StatusOK, test.ReceivedResponse) defer server.Server.Close() - client := New(Config{ - Host: server.Server.URL, - UserID: "u1", - DeviceID: "d1", - Log: log, - }) + client := newClient(server.Server.URL) client.Connect() err := client.Disconnect() @@ -129,3 +116,12 @@ func TestDisconnect(t *testing.T) { }) } } + +func newClient(url string) *Client { + return New(Config{ + Host: url, + UserID: "u1", + DeviceID: "d1", + Log: zap.NewNop(), + }) +} From 8ef0e2e7b2ef09d98c94ab18c35115b5df183892 Mon Sep 17 00:00:00 2001 From: Nickolay Kondratenko Date: Tue, 15 Apr 2025 15:43:01 +0300 Subject: [PATCH 5/6] make disconnect test more readable --- internal/client/client_test.go | 73 +++++++++++++++------------------- 1 file changed, 31 insertions(+), 42 deletions(-) diff --git a/internal/client/client_test.go b/internal/client/client_test.go index 9572569..e9e79c0 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -67,54 +67,43 @@ func TestConnect(t *testing.T) { } func TestDisconnect(t *testing.T) { - tests := []struct { - Name string - ReceivedResponse int - IsError bool - Heartbeat bool - }{ - { - Name: "Success", - ReceivedResponse: http.StatusOK, - IsError: false, - Heartbeat: false, - }, - { - Name: "Fail", - ReceivedResponse: http.StatusInternalServerError, - IsError: true, - Heartbeat: true, - }, - } + t.Run("Successful disconnect", func(t *testing.T) { + t.Parallel() - for _, test := range tests { - t.Run(test.Name, func(t *testing.T) { - t.Parallel() + server := newMockServer(http.StatusOK, http.StatusOK) + defer server.Server.Close() - server := newMockServer(http.StatusOK, test.ReceivedResponse) - defer server.Server.Close() + client := newClient(server.Server.URL) - client := newClient(server.Server.URL) + client.Connect() + err := client.Disconnect() + assert.NoError(t, err) - client.Connect() - err := client.Disconnect() - if test.IsError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } + time.Sleep(2 * time.Second) - time.Sleep(2 * time.Second) + server.mu.Lock() + assert.Zero(t, server.HeartbeatCount) + server.mu.Unlock() + }) - server.mu.Lock() - if test.Heartbeat { - assert.GreaterOrEqual(t, server.HeartbeatCount, 1) - } else { - assert.Zero(t, server.HeartbeatCount) - } - server.mu.Unlock() - }) - } + t.Run("Failed disconnect", func(t *testing.T) { + t.Parallel() + + server := newMockServer(http.StatusOK, http.StatusInternalServerError) + defer server.Server.Close() + + client := newClient(server.Server.URL) + + client.Connect() + err := client.Disconnect() + assert.Error(t, err) + + time.Sleep(2 * time.Second) + + server.mu.Lock() + assert.GreaterOrEqual(t, server.HeartbeatCount, 1) + server.mu.Unlock() + }) } func newClient(url string) *Client { From a53b962bc7ee2f3e13f229b7d0e5e543ba4ba079 Mon Sep 17 00:00:00 2001 From: Nickolay Kondratenko Date: Tue, 15 Apr 2025 16:06:31 +0300 Subject: [PATCH 6/6] make connect tests more readable --- internal/client/client_test.go | 106 ++++++++++++++++----------------- 1 file changed, 53 insertions(+), 53 deletions(-) diff --git a/internal/client/client_test.go b/internal/client/client_test.go index e9e79c0..644ca4b 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -11,59 +11,59 @@ import ( ) func TestConnect(t *testing.T) { - tests := []struct { - Name string - ReceivedResponse int - IsError bool - Heartbeat bool - }{ - { - Name: "Server returns 500", - ReceivedResponse: http.StatusInternalServerError, - IsError: true, - Heartbeat: false, - }, - { - Name: "Connection already exists", - ReceivedResponse: http.StatusConflict, - IsError: true, - Heartbeat: false, - }, - { - Name: "Connected successfully", - ReceivedResponse: http.StatusOK, - IsError: false, - Heartbeat: true, - }, - } - - for _, test := range tests { - t.Run(test.Name, func(t *testing.T) { - t.Parallel() - - server := newMockServer(test.ReceivedResponse, http.StatusOK) - defer server.Server.Close() - - client := newClient(server.Server.URL) - - err := client.Connect() - if test.IsError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - } - - time.Sleep(2 * time.Second) - - server.mu.Lock() - if test.Heartbeat { - assert.GreaterOrEqual(t, server.HeartbeatCount, 1) - } else { - assert.Zero(t, server.HeartbeatCount) - } - server.mu.Unlock() - }) - } + t.Run("Successful connect", func(t *testing.T) { + t.Parallel() + + server := newMockServer(http.StatusOK, http.StatusOK) + defer server.Server.Close() + + client := newClient(server.Server.URL) + + err := client.Connect() + assert.NoError(t, err) + + time.Sleep(2 * time.Second) + + server.mu.Lock() + assert.GreaterOrEqual(t, server.HeartbeatCount, 1) + server.mu.Unlock() + }) + + t.Run("Connection exists", func(t *testing.T) { + t.Parallel() + + server := newMockServer(http.StatusConflict, http.StatusOK) + defer server.Server.Close() + + client := newClient(server.Server.URL) + + err := client.Connect() + assert.Error(t, err) + + time.Sleep(2 * time.Second) + + server.mu.Lock() + assert.Zero(t, server.HeartbeatCount) + server.mu.Unlock() + }) + + t.Run("Internal server error", func(t *testing.T) { + t.Parallel() + + server := newMockServer(http.StatusInternalServerError, http.StatusOK) + defer server.Server.Close() + + client := newClient(server.Server.URL) + + err := client.Connect() + assert.Error(t, err) + + time.Sleep(2 * time.Second) + + server.mu.Lock() + assert.Zero(t, server.HeartbeatCount) + server.mu.Unlock() + }) } func TestDisconnect(t *testing.T) {