From 06daaf8d9f04c3f87f0baed47f6a5932f10d4545 Mon Sep 17 00:00:00 2001 From: Jakub Date: Thu, 14 Mar 2024 08:57:29 +0100 Subject: [PATCH] feat(GODT-3146): don't need to wait for IMAP in tests. --- internal/bridge/bridge_test.go | 54 --------------- internal/bridge/draft_test.go | 4 -- internal/bridge/send_test.go | 20 ------ internal/bridge/sentry_test.go | 5 -- internal/bridge/server_manager_test.go | 92 +++++++------------------- internal/bridge/user_event_test.go | 18 ----- tests/user_test.go | 31 ++------- 7 files changed, 29 insertions(+), 195 deletions(-) diff --git a/internal/bridge/bridge_test.go b/internal/bridge/bridge_test.go index 8b3b956c..492f75dc 100644 --- a/internal/bridge/bridge_test.go +++ b/internal/bridge/bridge_test.go @@ -184,20 +184,11 @@ func TestBridge_UserAgent_Persistence(t *testing.T) { require.NoError(t, err) withBridge(ctx, t, s.GetHostURL(), netCtl, locator, vaultKey, func(b *bridge.Bridge, mocks *bridge.Mocks) { - imapWaiter := waitForIMAPServerReady(b) - defer imapWaiter.Done() - - smtpWaiter := waitForSMTPServerReady(b) - defer smtpWaiter.Done() - currentUserAgent := b.GetCurrentUserAgent() require.Contains(t, currentUserAgent, useragent.DefaultUserAgent) require.NoError(t, getErr(b.LoginFull(ctx, otherUser, otherPassword, nil, nil))) - imapWaiter.Wait() - smtpWaiter.Wait() - imapClient, err := eventuallyDial(fmt.Sprintf("%v:%v", constants.Host, b.GetIMAPPort())) require.NoError(t, err) defer func() { _ = imapClient.Logout() }() @@ -235,21 +226,12 @@ func TestBridge_UserAgentFromUnknownClient(t *testing.T) { require.NoError(t, err) withBridge(ctx, t, s.GetHostURL(), netCtl, locator, vaultKey, func(b *bridge.Bridge, mocks *bridge.Mocks) { - imapWaiter := waitForIMAPServerReady(b) - defer imapWaiter.Done() - - smtpWaiter := waitForSMTPServerReady(b) - defer smtpWaiter.Done() - currentUserAgent := b.GetCurrentUserAgent() require.Contains(t, currentUserAgent, useragent.DefaultUserAgent) userID, err := b.LoginFull(context.Background(), username, password, nil, nil) require.NoError(t, err) - imapWaiter.Wait() - smtpWaiter.Wait() - imapClient, err := eventuallyDial(fmt.Sprintf("%v:%v", constants.Host, b.GetIMAPPort())) require.NoError(t, err) defer func() { _ = imapClient.Logout() }() @@ -274,21 +256,12 @@ func TestBridge_UserAgentFromSMTPClient(t *testing.T) { require.NoError(t, err) withBridge(ctx, t, s.GetHostURL(), netCtl, locator, vaultKey, func(b *bridge.Bridge, mocks *bridge.Mocks) { - imapWaiter := waitForIMAPServerReady(b) - defer imapWaiter.Done() - - smtpWaiter := waitForSMTPServerReady(b) - defer smtpWaiter.Done() - currentUserAgent := b.GetCurrentUserAgent() require.Contains(t, currentUserAgent, useragent.DefaultUserAgent) userID, err := b.LoginFull(context.Background(), username, password, nil, nil) require.NoError(t, err) - imapWaiter.Wait() - smtpWaiter.Wait() - client, err := smtp.Dial(net.JoinHostPort(constants.Host, fmt.Sprint(b.GetSMTPPort()))) require.NoError(t, err) defer client.Close() //nolint:errcheck @@ -333,17 +306,8 @@ func TestBridge_UserAgentFromIMAPID(t *testing.T) { require.NoError(t, err) withBridge(ctx, t, s.GetHostURL(), netCtl, locator, vaultKey, func(b *bridge.Bridge, mocks *bridge.Mocks) { - imapWaiter := waitForIMAPServerReady(b) - defer imapWaiter.Done() - - smtpWaiter := waitForSMTPServerReady(b) - defer smtpWaiter.Done() - require.NoError(t, getErr(b.LoginFull(ctx, otherUser, otherPassword, nil, nil))) - imapWaiter.Wait() - smtpWaiter.Wait() - imapClient, err := eventuallyDial(fmt.Sprintf("%v:%v", constants.Host, b.GetIMAPPort())) require.NoError(t, err) defer func() { _ = imapClient.Logout() }() @@ -715,21 +679,12 @@ func TestBridge_InitGluonDirectory(t *testing.T) { func TestBridge_LoginFailed(t *testing.T) { withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, vaultKey []byte) { withBridge(ctx, t, s.GetHostURL(), netCtl, locator, vaultKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) { - imapWaiter := waitForIMAPServerReady(bridge) - defer imapWaiter.Done() - - smtpWaiter := waitForSMTPServerReady(bridge) - defer smtpWaiter.Done() - failCh, done := chToType[events.Event, events.IMAPLoginFailed](bridge.GetEvents(events.IMAPLoginFailed{})) defer done() _, err := bridge.LoginFull(ctx, username, password, nil, nil) require.NoError(t, err) - imapWaiter.Wait() - smtpWaiter.Wait() - imapClient, err := eventuallyDial(net.JoinHostPort(constants.Host, fmt.Sprint(bridge.GetIMAPPort()))) require.NoError(t, err) @@ -757,12 +712,6 @@ func TestBridge_ChangeCacheDirectory(t *testing.T) { configDir, err := b.GetGluonDataDir() require.NoError(t, err) - imapWaiter := waitForIMAPServerReady(b) - defer imapWaiter.Done() - - smtpWaiter := waitForSMTPServerReady(b) - defer smtpWaiter.Done() - // Login the user. syncCh, done := chToType[events.Event, events.SyncFinished](b.GetEvents(events.SyncFinished{})) defer done() @@ -796,9 +745,6 @@ func TestBridge_ChangeCacheDirectory(t *testing.T) { require.NoError(t, err) require.True(t, info.State == bridge.Connected) - imapWaiter.Wait() - smtpWaiter.Wait() - client, err := eventuallyDial(fmt.Sprintf("%v:%v", constants.Host, b.GetIMAPPort())) require.NoError(t, err) require.NoError(t, client.Login(info.Addresses[0], string(info.BridgePass))) diff --git a/internal/bridge/draft_test.go b/internal/bridge/draft_test.go index dc609782..56585ca9 100644 --- a/internal/bridge/draft_test.go +++ b/internal/bridge/draft_test.go @@ -64,9 +64,6 @@ func TestBridge_HandleDraftsSendFromOtherClient(t *testing.T) { // The initial user should be fully synced. withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(b *bridge.Bridge, _ *bridge.Mocks) { - waiter := waitForIMAPServerReady(b) - defer waiter.Done() - syncCh, done := chToType[events.Event, events.SyncFinished](b.GetEvents(events.SyncFinished{})) defer done() @@ -74,7 +71,6 @@ func TestBridge_HandleDraftsSendFromOtherClient(t *testing.T) { require.NoError(t, err) require.Equal(t, userID, (<-syncCh).UserID) - waiter.Wait() info, err := b.GetUserInfo(userID) require.NoError(t, err) diff --git a/internal/bridge/send_test.go b/internal/bridge/send_test.go index 7914090e..30964715 100644 --- a/internal/bridge/send_test.go +++ b/internal/bridge/send_test.go @@ -46,17 +46,12 @@ func TestBridge_Send(t *testing.T) { require.NoError(t, err) withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, _ *bridge.Mocks) { - smtpWaiter := waitForSMTPServerReady(bridge) - defer smtpWaiter.Done() - senderUserID, err := bridge.LoginFull(ctx, username, password, nil, nil) require.NoError(t, err) recipientUserID, err := bridge.LoginFull(ctx, "recipient", password, nil, nil) require.NoError(t, err) - smtpWaiter.Wait() - senderInfo, err := bridge.GetUserInfo(senderUserID) require.NoError(t, err) @@ -409,9 +404,6 @@ SGVsbG8gd29ybGQK require.NoError(t, err) withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, _ *bridge.Mocks) { - smtpWaiter := waitForSMTPServerReady(bridge) - defer smtpWaiter.Done() - senderUserID, err := bridge.LoginFull(ctx, username, password, nil, nil) require.NoError(t, err) @@ -431,8 +423,6 @@ SGVsbG8gd29ybGQK messageMultipartWithoutTextWithTextAttachment, } - smtpWaiter.Wait() - for _, m := range messages { // Dial the server. client, err := smtp.Dial(net.JoinHostPort(constants.Host, fmt.Sprint(bridge.GetSMTPPort()))) @@ -617,9 +607,6 @@ Hello world require.NoError(t, err) withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, _ *bridge.Mocks) { - smtpWaiter := waitForSMTPServerReady(bridge) - defer smtpWaiter.Done() - senderUserID, err := bridge.LoginFull(ctx, username, password, nil, nil) require.NoError(t, err) @@ -639,8 +626,6 @@ Hello world messageInlineImageFollowedByText, } - smtpWaiter.Wait() - for _, m := range messages { // Dial the server. client, err := smtp.Dial(net.JoinHostPort(constants.Host, fmt.Sprint(bridge.GetSMTPPort()))) @@ -714,17 +699,12 @@ func TestBridge_SendAddressDisabled(t *testing.T) { require.NoError(t, s.ChangeAddressAllowSend(senderUserID, addrID, false)) withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, _ *bridge.Mocks) { - smtpWaiter := waitForSMTPServerReady(bridge) - defer smtpWaiter.Done() - senderUserID, err := bridge.LoginFull(ctx, "sender", password, nil, nil) require.NoError(t, err) _, err = bridge.LoginFull(ctx, "recipient", password, nil, nil) require.NoError(t, err) - smtpWaiter.Wait() - recipientInfo, err := bridge.GetUserInfo(recipientUserID) require.NoError(t, err) diff --git a/internal/bridge/sentry_test.go b/internal/bridge/sentry_test.go index 54fa0d35..2083ed90 100644 --- a/internal/bridge/sentry_test.go +++ b/internal/bridge/sentry_test.go @@ -36,9 +36,6 @@ import ( func TestBridge_Report(t *testing.T) { withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, storeKey []byte) { withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(b *bridge.Bridge, mocks *bridge.Mocks) { - imapWaiter := waitForIMAPServerReady(b) - defer imapWaiter.Done() - syncCh, done := chToType[events.Event, events.SyncFinished](b.GetEvents(events.SyncFinished{})) defer done() @@ -54,8 +51,6 @@ func TestBridge_Report(t *testing.T) { require.NoError(t, err) require.True(t, info.State == bridge.Connected) - imapWaiter.Wait() - // Dial the IMAP port. conn, err := net.Dial("tcp", fmt.Sprintf("%v:%v", constants.Host, b.GetIMAPPort())) require.NoError(t, err) diff --git a/internal/bridge/server_manager_test.go b/internal/bridge/server_manager_test.go index e597c2aa..ee39590e 100644 --- a/internal/bridge/server_manager_test.go +++ b/internal/bridge/server_manager_test.go @@ -20,6 +20,7 @@ package bridge_test import ( "context" "fmt" + "net" "testing" "github.com/ProtonMail/go-proton-api" @@ -27,57 +28,39 @@ import ( "github.com/ProtonMail/proton-bridge/v3/internal/bridge" "github.com/ProtonMail/proton-bridge/v3/internal/constants" "github.com/ProtonMail/proton-bridge/v3/internal/events" + "github.com/emersion/go-smtp" "github.com/stretchr/testify/require" ) -func TestServerManager_NoLoadedUsersNoServers(t *testing.T) { +func TestServerManager_ServersStartWithBridge(t *testing.T) { withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, storeKey []byte) { withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) { - _, err := eventuallyDial(fmt.Sprintf("%v:%v", constants.Host, bridge.GetIMAPPort())) - require.Error(t, err) - }) - }) -} - -func TestServerManager_ServersStartAfterFirstConnectedUser(t *testing.T) { - withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, storeKey []byte) { - withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) { - imapWaiter := waitForIMAPServerReady(bridge) - defer imapWaiter.Done() - - smtpWaiter := waitForSMTPServerReady(bridge) - defer smtpWaiter.Done() - - _, err := bridge.LoginFull(ctx, username, password, nil, nil) + imapClient, err := eventuallyDial(fmt.Sprintf("%v:%v", constants.Host, bridge.GetIMAPPort())) require.NoError(t, err) + require.NoError(t, imapClient.Logout()) - imapWaiter.Wait() - smtpWaiter.Wait() + smtpClient, err := smtp.Dial(net.JoinHostPort(constants.Host, fmt.Sprint(bridge.GetSMTPPort()))) + require.NoError(t, err) + smtpClient.Close() //nolint:errcheck }) }) } -func TestServerManager_ServersStopsAfterUserLogsOut(t *testing.T) { +func TestServerManager_ServersKeepsRunningfterUserLogsOut(t *testing.T) { withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, storeKey []byte) { withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) { - imapWaiter := waitForIMAPServerReady(bridge) - defer imapWaiter.Done() - - smtpWaiter := waitForSMTPServerReady(bridge) - defer smtpWaiter.Done() - userID, err := bridge.LoginFull(ctx, username, password, nil, nil) require.NoError(t, err) - imapWaiter.Wait() - smtpWaiter.Wait() - - imapWaiterStopped := waitForIMAPServerStopped(bridge) - defer imapWaiterStopped.Done() - require.NoError(t, bridge.LogoutUser(ctx, userID)) - imapWaiterStopped.Wait() + imapClient, err := eventuallyDial(fmt.Sprintf("%v:%v", constants.Host, bridge.GetIMAPPort())) + require.NoError(t, err) + require.NoError(t, imapClient.Logout()) + + smtpClient, err := smtp.Dial(net.JoinHostPort(constants.Host, fmt.Sprint(bridge.GetSMTPPort()))) + require.NoError(t, err) + smtpClient.Close() //nolint:errcheck }) }) } @@ -90,21 +73,12 @@ func TestServerManager_ServersDoNotStopWhenThereIsStillOneActiveUser(t *testing. require.NoError(t, err) withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) { - imapWaiter := waitForIMAPServerReady(bridge) - defer imapWaiter.Done() - - smtpWaiter := waitForSMTPServerReady(bridge) - defer smtpWaiter.Done() - _, err := bridge.LoginFull(ctx, username, password, nil, nil) require.NoError(t, err) userIDOther, err := bridge.LoginFull(ctx, otherUser, otherPassword, nil, nil) require.NoError(t, err) - imapWaiter.Wait() - smtpWaiter.Wait() - evtCh, cancel := bridge.GetEvents(events.UserDeauth{}) defer cancel() @@ -115,31 +89,10 @@ func TestServerManager_ServersDoNotStopWhenThereIsStillOneActiveUser(t *testing. imapClient, err := eventuallyDial(fmt.Sprintf("%v:%v", constants.Host, bridge.GetIMAPPort())) require.NoError(t, err) require.NoError(t, imapClient.Logout()) - }) - }) -} -func TestServerManager_ServersStartIfAtLeastOneUserIsLoggedIn(t *testing.T) { - otherPassword := []byte("bar") - otherUser := "foo" - withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, storeKey []byte) { - userIDOther, _, err := s.CreateUser(otherUser, otherPassword) - require.NoError(t, err) - - withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) { - _, err := bridge.LoginFull(ctx, username, password, nil, nil) + smtpClient, err := smtp.Dial(net.JoinHostPort(constants.Host, fmt.Sprint(bridge.GetSMTPPort()))) require.NoError(t, err) - - _, err = bridge.LoginFull(ctx, otherUser, otherPassword, nil, nil) - require.NoError(t, err) - }) - - require.NoError(t, s.RevokeUser(userIDOther)) - - withBridgeWaitForServers(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) { - imapClient, err := eventuallyDial(fmt.Sprintf("%v:%v", constants.Host, bridge.GetIMAPPort())) - require.NoError(t, err) - require.NoError(t, imapClient.Logout()) + smtpClient.Close() //nolint:errcheck }) }) } @@ -162,8 +115,13 @@ func TestServerManager_NetworkLossStopsServers(t *testing.T) { _, err := bridge.LoginFull(ctx, username, password, nil, nil) require.NoError(t, err) - imapWaiter.Wait() - smtpWaiter.Wait() + imapClient, err := eventuallyDial(fmt.Sprintf("%v:%v", constants.Host, bridge.GetIMAPPort())) + require.NoError(t, err) + require.NoError(t, imapClient.Logout()) + + smtpClient, err := smtp.Dial(net.JoinHostPort(constants.Host, fmt.Sprint(bridge.GetSMTPPort()))) + require.NoError(t, err) + smtpClient.Close() //nolint:errcheck netCtl.Disable() diff --git a/internal/bridge/user_event_test.go b/internal/bridge/user_event_test.go index 95b413ba..09d39afb 100644 --- a/internal/bridge/user_event_test.go +++ b/internal/bridge/user_event_test.go @@ -139,9 +139,6 @@ func test_badMessage_badEvent(userFeedback func(t *testing.T, ctx context.Contex }) withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) { - smtpWaiter := waitForSMTPServerReady(bridge) - defer smtpWaiter.Done() - userLoginAndSync(ctx, t, bridge, "user", password) var messageIDs []string @@ -177,8 +174,6 @@ func test_badMessage_badEvent(userFeedback func(t *testing.T, ctx context.Contex userFeedback(t, ctx, bridge, badUserID) - smtpWaiter.Wait() - userContinueEventProcess(ctx, t, s, bridge) }) }) @@ -197,9 +192,6 @@ func TestBridge_User_BadMessage_NoBadEvent(t *testing.T) { }) withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) { - smtpWaiter := waitForSMTPServerReady(bridge) - defer smtpWaiter.Done() - userLoginAndSync(ctx, t, bridge, "user", password) var messageIDs []string @@ -223,7 +215,6 @@ func TestBridge_User_BadMessage_NoBadEvent(t *testing.T) { require.NoError(t, c.DeleteMessage(ctx, messageIDs...)) }) - smtpWaiter.Wait() userContinueEventProcess(ctx, t, s, bridge) }) }) @@ -776,20 +767,11 @@ func TestBridge_User_CreateDisabledAddress(t *testing.T) { func TestBridge_User_HandleParentLabelRename(t *testing.T) { withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, storeKey []byte) { withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, mocks *bridge.Mocks) { - imapWaiter := waitForIMAPServerReady(bridge) - defer imapWaiter.Done() - - smtpWaiter := waitForSMTPServerReady(bridge) - defer smtpWaiter.Done() - require.NoError(t, getErr(bridge.LoginFull(ctx, username, password, nil, nil))) info, err := bridge.QueryUserInfo(username) require.NoError(t, err) - imapWaiter.Wait() - smtpWaiter.Wait() - cli, err := eventuallyDial(fmt.Sprintf("%v:%v", constants.Host, bridge.GetIMAPPort())) require.NoError(t, err) require.NoError(t, cli.Login(info.Addresses[0], string(info.BridgePass))) diff --git a/tests/user_test.go b/tests/user_test.go index eeb0c4b8..5030e534 100644 --- a/tests/user_test.go +++ b/tests/user_test.go @@ -28,7 +28,6 @@ import ( "github.com/ProtonMail/go-proton-api" "github.com/ProtonMail/gopenpgp/v2/crypto" "github.com/ProtonMail/proton-bridge/v3/internal/bridge" - "github.com/ProtonMail/proton-bridge/v3/internal/events" "github.com/ProtonMail/proton-bridge/v3/internal/vault" "github.com/ProtonMail/proton-bridge/v3/pkg/algo" "github.com/bradenaw/juniper/iterator" @@ -330,25 +329,14 @@ func (s *scenario) drafAtIndexWasMovedToTrashForAddressOfAccount(draftIndex int, } func (s *scenario) userLogsInWithUsernameAndPassword(username, password string) error { - smtpEvtCh, cancelSMTP := s.t.bridge.GetEvents(events.SMTPServerReady{}) - defer cancelSMTP() - imapEvtCh, cancelIMAP := s.t.bridge.GetEvents(events.IMAPServerReady{}) - defer cancelIMAP() - userID, err := s.t.bridge.LoginFull(context.Background(), username, []byte(password), nil, nil) if err != nil { s.t.pushError(err) } else { // We need to wait for server to be up or we won't be able to connect. It should only happen once to avoid // blocking on multiple Logins. - if !s.t.imapServerStarted { - <-imapEvtCh - s.t.imapServerStarted = true - } - if !s.t.smtpServerStarted { - <-smtpEvtCh - s.t.smtpServerStarted = true - } + s.t.imapServerStarted = true + s.t.smtpServerStarted = true if userID != s.t.getUserByName(username).getUserID() { return errors.New("user ID mismatch") @@ -366,25 +354,14 @@ func (s *scenario) userLogsInWithUsernameAndPassword(username, password string) } func (s *scenario) userLogsInWithAliasAddressAndPassword(alias, password string) error { - smtpEvtCh, cancelSMTP := s.t.bridge.GetEvents(events.SMTPServerReady{}) - defer cancelSMTP() - imapEvtCh, cancelIMAP := s.t.bridge.GetEvents(events.IMAPServerReady{}) - defer cancelIMAP() - userID, err := s.t.bridge.LoginFull(context.Background(), s.t.getUserByAddress(alias).getName(), []byte(password), nil, nil) if err != nil { s.t.pushError(err) } else { // We need to wait for server to be up or we won't be able to connect. It should only happen once to avoid // blocking on multiple Logins. - if !s.t.imapServerStarted { - <-imapEvtCh - s.t.imapServerStarted = true - } - if !s.t.smtpServerStarted { - <-smtpEvtCh - s.t.smtpServerStarted = true - } + s.t.imapServerStarted = true + s.t.smtpServerStarted = true if userID != s.t.getUserByAddress(alias).getUserID() { return errors.New("user ID mismatch")