From 951d591f4fdaad39bc5e270b7c74e4e8ba562710 Mon Sep 17 00:00:00 2001 From: kim Date: Thu, 25 Apr 2024 19:26:43 +0100 Subject: [PATCH] fix account test suite queue popping logic, ensure noop workers do not pull from queue --- internal/processing/account/account_test.go | 11 +++++++++ internal/processing/account/follow_test.go | 3 ++- internal/processing/account/move_test.go | 2 +- internal/processing/account/update_test.go | 25 +++++---------------- testrig/util.go | 24 +++++++++----------- 5 files changed, 30 insertions(+), 35 deletions(-) diff --git a/internal/processing/account/account_test.go b/internal/processing/account/account_test.go index c41ea4551..10d5f91e1 100644 --- a/internal/processing/account/account_test.go +++ b/internal/processing/account/account_test.go @@ -18,6 +18,9 @@ package account_test import ( + "context" + "time" + "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/email" @@ -25,6 +28,7 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/filter/visibility" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/media" + "github.com/superseriousbusiness/gotosocial/internal/messages" "github.com/superseriousbusiness/gotosocial/internal/oauth" "github.com/superseriousbusiness/gotosocial/internal/processing" "github.com/superseriousbusiness/gotosocial/internal/processing/account" @@ -64,6 +68,13 @@ type AccountStandardTestSuite struct { accountProcessor account.Processor } +func (suite *AccountStandardTestSuite) getClientMsg(timeout time.Duration) (*messages.FromClientAPI, bool) { + ctx := context.Background() + ctx, cncl := context.WithTimeout(ctx, timeout) + defer cncl() + return suite.state.Workers.Client.Queue.PopCtx(ctx) +} + func (suite *AccountStandardTestSuite) SetupSuite() { suite.testTokens = testrig.NewTestTokens() suite.testClients = testrig.NewTestClients() diff --git a/internal/processing/account/follow_test.go b/internal/processing/account/follow_test.go index b8e69f9d6..9ea8ce1b8 100644 --- a/internal/processing/account/follow_test.go +++ b/internal/processing/account/follow_test.go @@ -20,6 +20,7 @@ package account_test import ( "context" "testing" + "time" "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/gotosocial/internal/ap" @@ -150,7 +151,7 @@ func (suite *FollowTestSuite) TestFollowRequestLocal() { } // There should be a message going to the worker. - cMsg := suite.checkClientAPIChan() + cMsg, _ := suite.getClientMsg(5 * time.Second) suite.Equal(ap.ActivityCreate, cMsg.APActivityType) suite.Equal(ap.ActivityFollow, cMsg.APObjectType) suite.Equal(requestingAccount.ID, cMsg.Origin.ID) diff --git a/internal/processing/account/move_test.go b/internal/processing/account/move_test.go index e2f4b6fd9..c1a931252 100644 --- a/internal/processing/account/move_test.go +++ b/internal/processing/account/move_test.go @@ -71,7 +71,7 @@ func (suite *MoveTestSuite) TestMoveAccountOK() { } // There should be a message going to the worker. - cMsg := suite.checkClientAPIChan() + cMsg, _ := suite.getClientMsg(5 * time.Second) move, ok := cMsg.GTSModel.(*gtsmodel.Move) if !ok { suite.FailNow("", "could not cast %T to *gtsmodel.Move", move) diff --git a/internal/processing/account/update_test.go b/internal/processing/account/update_test.go index 21859eca4..ad09ff25c 100644 --- a/internal/processing/account/update_test.go +++ b/internal/processing/account/update_test.go @@ -26,27 +26,12 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/ap" apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" - "github.com/superseriousbusiness/gotosocial/internal/messages" ) type AccountUpdateTestSuite struct { AccountStandardTestSuite } -func (suite *AccountStandardTestSuite) checkClientAPIChan() *messages.FromClientAPI { - select { - case <-suite.state.Workers.Client.Queue.Wait(): - case <-time.After(5 * time.Second): - } - - msg, ok := suite.state.Workers.Client.Queue.Pop() - if !ok { - suite.FailNow("no queued message") - } - - return msg -} - func (suite *AccountUpdateTestSuite) TestAccountUpdateSimple() { testAccount := >smodel.Account{} *testAccount = *suite.testAccounts["local_account_1"] @@ -75,7 +60,7 @@ func (suite *AccountUpdateTestSuite) TestAccountUpdateSimple() { suite.Equal(noteExpected, apiAccount.Note) // We should have an update in the client api channel. - msg := suite.checkClientAPIChan() + msg, _ := suite.getClientMsg(5 * time.Second) // Profile update. suite.Equal(ap.ActivityUpdate, msg.APActivityType) @@ -125,7 +110,7 @@ func (suite *AccountUpdateTestSuite) TestAccountUpdateWithMention() { suite.Equal(noteExpected, apiAccount.Note) // We should have an update in the client api channel. - msg := suite.checkClientAPIChan() + msg, _ := suite.getClientMsg(5 * time.Second) // Profile update. suite.Equal(ap.ActivityUpdate, msg.APActivityType) @@ -181,7 +166,7 @@ func (suite *AccountUpdateTestSuite) TestAccountUpdateWithMarkdownNote() { suite.Equal(noteExpected, apiAccount.Note) // We should have an update in the client api channel. - msg := suite.checkClientAPIChan() + msg, _ := suite.getClientMsg(5 * time.Second) // Profile update. suite.Equal(ap.ActivityUpdate, msg.APActivityType) @@ -266,7 +251,7 @@ func (suite *AccountUpdateTestSuite) TestAccountUpdateWithFields() { suite.EqualValues(emojisExpected, apiAccount.Emojis) // We should have an update in the client api channel. - msg := suite.checkClientAPIChan() + msg, _ := suite.getClientMsg(5 * time.Second) // Profile update. suite.Equal(ap.ActivityUpdate, msg.APActivityType) @@ -323,7 +308,7 @@ func (suite *AccountUpdateTestSuite) TestAccountUpdateNoteNotFields() { suite.Equal(fieldsBefore, len(apiAccount.Fields)) // We should have an update in the client api channel. - msg := suite.checkClientAPIChan() + msg, _ := suite.getClientMsg(5 * time.Second) // Profile update. suite.Equal(ap.ActivityUpdate, msg.APActivityType) diff --git a/testrig/util.go b/testrig/util.go index 5947e42de..ad4ffcb6c 100644 --- a/testrig/util.go +++ b/testrig/util.go @@ -42,25 +42,23 @@ import ( // Starts workers on the provided state using noop processing functions. // Useful when you *don't* want to trigger side effects in a test. func StartNoopWorkers(state *state.State) { - state.Workers.Client.Process = func(ctx context.Context, msg *messages.FromClientAPI) error { - log.Debugf(ctx, "Workers{}.Client{}.Noop(%s)", dump(msg)) - return nil // noop - } - - state.Workers.Federator.Process = func(ctx context.Context, msg *messages.FromFediAPI) error { - log.Debugf(ctx, "Workers{}.Federator{}.Noop(%s)", dump(msg)) - return nil // noop - } + state.Workers.Client.Process = func(ctx context.Context, msg *messages.FromClientAPI) error { return nil } + state.Workers.Federator.Process = func(ctx context.Context, msg *messages.FromFediAPI) error { return nil } state.Workers.Client.Init(messages.ClientMsgIndices()) state.Workers.Federator.Init(messages.FederatorMsgIndices()) state.Workers.Delivery.Init(nil) + // Specifically do NOT start the workers + // as caller may require queue contents. + // (i.e. don't want workers pulling) + // _ = state.Workers.Client.Start(1) + // _ = state.Workers.Federator.Start(1) + // _ = state.Workers.Dereference.Start(1) + // _ = state.Workers.Media.Start(1) + // + // (except for the scheduler, that's fine) _ = state.Workers.Scheduler.Start() - _ = state.Workers.Client.Start(1) - _ = state.Workers.Federator.Start(1) - _ = state.Workers.Dereference.Start(1) - _ = state.Workers.Media.Start(1) } // Starts workers on the provided state using processing functions from the given