From 04313b63c7f2fde6d945327ce606993dd3dcfd62 Mon Sep 17 00:00:00 2001 From: Martin Englund Date: Sat, 30 Dec 2023 20:18:26 -0800 Subject: [PATCH 1/3] return error when calling stop without first calling start --- client.go | 4 ++++ client_test.go | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/client.go b/client.go index fecc8a33..97fd03bc 100644 --- a/client.go +++ b/client.go @@ -682,6 +682,10 @@ func (c *Client[TTx]) signalStopComplete(ctx context.Context) { // by cancelling the context passed to Start or by calling StopAndCancel. func (c *Client[TTx]) Stop(ctx context.Context) error { c.baseService.Logger.InfoContext(ctx, c.baseService.Name+": Stop started") + if c.fetchNewWorkCancel != nil { + return errors.New("client not started") + } + c.fetchNewWorkCancel() return c.awaitStop(ctx) } diff --git a/client_test.go b/client_test.go index 09b05778..74dabbec 100644 --- a/client_test.go +++ b/client_test.go @@ -17,6 +17,7 @@ import ( "github.com/jackc/pgx/v5/pgconn" "github.com/jackc/pgx/v5/pgxpool" "github.com/robfig/cron/v3" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/riverqueue/river/internal/componentstatus" @@ -389,6 +390,15 @@ func Test_Client_Stop(t *testing.T) { } } + t.Run("not started", func(t *testing.T) { + t.Parallel() + client := newTestClient(ctx, t, newTestConfig(t, nil)) + + err := client.Stop(ctx) + require.Error(t, err) + assert.Equal(t, "client not started", err.Error()) + }) + t.Run("no jobs in progress", func(t *testing.T) { t.Parallel() client := runNewTestClient(ctx, t, newTestConfig(t, nil)) From 08a3df2a9514cde78ff101385261530014d11d54 Mon Sep 17 00:00:00 2001 From: Martin Englund Date: Sat, 30 Dec 2023 21:37:39 -0800 Subject: [PATCH 2/3] address code review feedback --- client.go | 2 +- client_test.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/client.go b/client.go index 97fd03bc..60ad7923 100644 --- a/client.go +++ b/client.go @@ -681,11 +681,11 @@ func (c *Client[TTx]) signalStopComplete(ctx context.Context) { // There's no need to call this method if a hard stop has already been initiated // by cancelling the context passed to Start or by calling StopAndCancel. func (c *Client[TTx]) Stop(ctx context.Context) error { - c.baseService.Logger.InfoContext(ctx, c.baseService.Name+": Stop started") if c.fetchNewWorkCancel != nil { return errors.New("client not started") } + c.baseService.Logger.InfoContext(ctx, c.baseService.Name+": Stop started") c.fetchNewWorkCancel() return c.awaitStop(ctx) } diff --git a/client_test.go b/client_test.go index 74dabbec..733fe6ff 100644 --- a/client_test.go +++ b/client_test.go @@ -17,7 +17,6 @@ import ( "github.com/jackc/pgx/v5/pgconn" "github.com/jackc/pgx/v5/pgxpool" "github.com/robfig/cron/v3" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/riverqueue/river/internal/componentstatus" @@ -396,7 +395,7 @@ func Test_Client_Stop(t *testing.T) { err := client.Stop(ctx) require.Error(t, err) - assert.Equal(t, "client not started", err.Error()) + require.Equal(t, "client not started", err.Error()) }) t.Run("no jobs in progress", func(t *testing.T) { From 85672cfc100e36e945774f256f3a88a6983d0357 Mon Sep 17 00:00:00 2001 From: Martin Englund Date: Sat, 30 Dec 2023 21:55:28 -0800 Subject: [PATCH 3/3] reverse predicate --- client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client.go b/client.go index 60ad7923..9245e3fd 100644 --- a/client.go +++ b/client.go @@ -681,7 +681,7 @@ func (c *Client[TTx]) signalStopComplete(ctx context.Context) { // There's no need to call this method if a hard stop has already been initiated // by cancelling the context passed to Start or by calling StopAndCancel. func (c *Client[TTx]) Stop(ctx context.Context) error { - if c.fetchNewWorkCancel != nil { + if c.fetchNewWorkCancel == nil { return errors.New("client not started") }