diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index fbce92e9..afaa780c 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -45,6 +45,8 @@ lint: - branches script: - make lint + tags: + - medium test: stage: test @@ -60,6 +62,8 @@ test: - pass init `gpg --list-keys | grep "^ " | tail -1 | tr -d '[:space:]'` # Then finally run the tests - make test + tags: + - medium test-integration: stage: test diff --git a/internal/imap/cache/cache_test.go b/internal/imap/cache/cache_test.go index 1401d964..2d43a0d8 100644 --- a/internal/imap/cache/cache_test.go +++ b/internal/imap/cache/cache_test.go @@ -18,7 +18,6 @@ package cache import ( - "bytes" "fmt" "testing" "time" @@ -59,34 +58,34 @@ func TestClearOld(t *testing.T) { } func TestClearBig(t *testing.T) { - msg := []byte("Test message") + r := require.New(t) + wantMessage := []byte("Test message") - nSize := 3 - cacheSizeLimit = nSize*len(msg) + 1 - cacheTimeLimit = int64(nSize * nSize * 2) // be sure the message will survive + wantCacheSize := 3 + nTestMessages := wantCacheSize * wantCacheSize + cacheSizeLimit = wantCacheSize*len(wantMessage) + 1 + cacheTimeLimit = int64(1 << 20) // be sure the message will survive - // It should have more than nSize items. - for i := 0; i < nSize*nSize; i++ { + // It should never have more than nSize items. + for i := 0; i < nTestMessages; i++ { time.Sleep(1 * time.Millisecond) - SaveMail(fmt.Sprintf("%s%d", testUID, i), msg, bs) - if len(mailCache) > nSize { - t.Error("Number of items in cache should not be more than", nSize) - } + SaveMail(fmt.Sprintf("%s%d", testUID, i), wantMessage, bs) + r.LessOrEqual(len(mailCache), wantCacheSize, "cache too big when %d", i) } // Check that the oldest are deleted first. - for i := 0; i < nSize*nSize; i++ { + for i := 0; i < nTestMessages; i++ { iUID := fmt.Sprintf("%s%d", testUID, i) reader, _ := LoadMail(iUID) - if i < nSize*(nSize-1) && reader.Len() != 0 { - mail := mailCache[iUID] - t.Error("LoadMail should return empty but have:", mail.data, iUID, mail.key.Timestamp) - } - stored := make([]byte, len(msg)) - _, _ = reader.Read(stored) + mail := mailCache[iUID] - if i >= nSize*(nSize-1) && !bytes.Equal(stored, msg) { - t.Error("LoadMail returned wrong message:", stored, iUID) + if i < (nTestMessages - wantCacheSize) { + r.Zero(reader.Len(), "LoadMail should return empty, but have %s for %s time %d ", string(mail.data), iUID, mail.key.Timestamp) + } else { + stored := make([]byte, len(wantMessage)) + _, err := reader.Read(stored) + r.NoError(err) + r.Equal(wantMessage, stored, "LoadMail returned wrong message: %s for %s time %d", stored, iUID, mail.key.Timestamp) } } } diff --git a/internal/store/event_loop_test.go b/internal/store/event_loop_test.go index 51751af8..90ca3c4c 100644 --- a/internal/store/event_loop_test.go +++ b/internal/store/event_loop_test.go @@ -24,7 +24,6 @@ import ( "github.com/ProtonMail/proton-bridge/pkg/pmapi" "github.com/golang/mock/gomock" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -54,10 +53,10 @@ func TestEventLoopProcessMoreEvents(t *testing.T) { }, nil), ) m.newStoreNoEvents(true) - m.client.EXPECT().ListMessages(gomock.Any()).Return([]*pmapi.Message{}, 0, nil).AnyTimes() - // Event loop runs in goroutine and will be stopped by deferred mock clearing. - go m.store.eventLoop.start() + // Event loop runs in goroutine started during store creation (newStoreNoEvents). + // Force to run the next event. + m.store.eventLoop.pollNow() // More events are processed right away. require.Eventually(t, func() bool { @@ -78,38 +77,42 @@ func TestEventLoopUpdateMessageFromLoop(t *testing.T) { subject := "old subject" newSubject := "new subject" - // First sync will add message with old subject to database. - m.client.EXPECT().GetMessage("msg1").Return(&pmapi.Message{ + m.newStoreNoEvents(true, &pmapi.Message{ ID: "msg1", Subject: subject, - }, nil) - // Event will update the subject. - m.client.EXPECT().GetEvent("latestEventID").Return(&pmapi.Event{ - EventID: "event1", - Messages: []*pmapi.EventMessage{{ - EventItem: pmapi.EventItem{ - ID: "msg1", - Action: pmapi.EventUpdate, - }, - Updated: &pmapi.EventMessageUpdated{ - ID: "msg1", - Subject: &newSubject, - }, - }}, - }, nil) + }) - m.newStoreNoEvents(true) + eventReceived := make(chan struct{}) + m.client.EXPECT().GetEvent("latestEventID").DoAndReturn(func(eventID string) (*pmapi.Event, error) { + defer close(eventReceived) + return &pmapi.Event{ + EventID: "event1", + Messages: []*pmapi.EventMessage{{ + EventItem: pmapi.EventItem{ + ID: "msg1", + Action: pmapi.EventUpdate, + }, + Updated: &pmapi.EventMessageUpdated{ + ID: "msg1", + Subject: &newSubject, + }, + }}, + }, nil + }) - // Event loop runs in goroutine and will be stopped by deferred mock clearing. - go m.store.eventLoop.start() + // Event loop runs in goroutine started during store creation (newStoreNoEvents). + // Force to run the next event. + m.store.eventLoop.pollNow() - var err error - assert.Eventually(t, func() bool { - var msg *pmapi.Message - msg, err = m.store.getMessageFromDB("msg1") - return err == nil && msg.Subject == newSubject - }, time.Second, 10*time.Millisecond) + select { + case <-eventReceived: + case <-time.After(5 * time.Second): + require.Fail(t, "latestEventID was not processed") + } + + msg, err := m.store.getMessageFromDB("msg1") require.NoError(t, err) + require.Equal(t, newSubject, msg.Subject) } func TestEventLoopUpdateMessage(t *testing.T) { diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 2957b936..ccf717fb 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -18,11 +18,12 @@ package store import ( + "fmt" "io/ioutil" "os" "path/filepath" - "sync" "testing" + "time" storemocks "github.com/ProtonMail/proton-bridge/internal/store/mocks" "github.com/ProtonMail/proton-bridge/pkg/pmapi" @@ -89,7 +90,7 @@ func initMocks(tb testing.TB) (*mocksForStore, func()) { } } -func (mocks *mocksForStore) newStoreNoEvents(combinedMode bool) { //nolint[unparam] +func (mocks *mocksForStore) newStoreNoEvents(combinedMode bool, msgs ...*pmapi.Message) { //nolint[unparam] mocks.user.EXPECT().ID().Return("userID").AnyTimes() mocks.user.EXPECT().IsConnected().Return(true) mocks.user.EXPECT().IsCombinedAddressMode().Return(combinedMode) @@ -102,20 +103,19 @@ func (mocks *mocksForStore) newStoreNoEvents(combinedMode bool) { //nolint[unpar }) mocks.client.EXPECT().ListLabels() mocks.client.EXPECT().CountMessages("") - mocks.client.EXPECT().GetEvent(gomock.Any()). - Return(&pmapi.Event{ - EventID: "latestEventID", - }, nil).AnyTimes() - // We want to wait until first sync has finished. - firstSyncWaiter := sync.WaitGroup{} - firstSyncWaiter.Add(1) - mocks.client.EXPECT(). - ListMessages(gomock.Any()). - DoAndReturn(func(*pmapi.MessagesFilter) ([]*pmapi.Message, int, error) { - firstSyncWaiter.Done() - return []*pmapi.Message{}, 0, nil - }) + // Call to get latest event ID and then to process first event. + mocks.client.EXPECT().GetEvent("").Return(&pmapi.Event{ + EventID: "firstEventID", + }, nil) + mocks.client.EXPECT().GetEvent("firstEventID").Return(&pmapi.Event{ + EventID: "latestEventID", + }, nil) + + mocks.client.EXPECT().ListMessages(gomock.Any()).Return(msgs, len(msgs), nil).AnyTimes() + for _, msg := range msgs { + mocks.client.EXPECT().GetMessage(msg.ID).Return(msg, nil).AnyTimes() + } var err error mocks.store, err = New( @@ -128,6 +128,16 @@ func (mocks *mocksForStore) newStoreNoEvents(combinedMode bool) { //nolint[unpar ) require.NoError(mocks.tb, err) - // Wait for sync to finish. - firstSyncWaiter.Wait() + // We want to wait until first sync has finished. + require.Eventually(mocks.tb, func() bool { + for _, msg := range msgs { + _, err := mocks.store.getMessageFromDB(msg.ID) + if err != nil { + // To see in test result the latest error for debugging. + fmt.Println("Sync wait error:", err) + return false + } + } + return true + }, 5*time.Second, 10*time.Millisecond) } diff --git a/internal/transfer/provider_test.go b/internal/transfer/provider_test.go index 62927425..61e5dace 100644 --- a/internal/transfer/provider_test.go +++ b/internal/transfer/provider_test.go @@ -85,7 +85,7 @@ func testTransferFrom(t *testing.T, rules transferRules, provider TargetProvider progress.finish() }() - maxWait := time.Duration(len(messages)) * time.Second + maxWait := time.Duration(len(messages)) * 2 * time.Second a.Eventually(t, func() bool { return progress.updateCh == nil }, maxWait, 10*time.Millisecond, "Waiting for imported messages timed out") diff --git a/internal/users/user_test.go b/internal/users/user_test.go index 73bcde1b..6d5285e3 100644 --- a/internal/users/user_test.go +++ b/internal/users/user_test.go @@ -20,8 +20,6 @@ package users import ( "testing" - "github.com/ProtonMail/proton-bridge/pkg/pmapi" - gomock "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -31,12 +29,7 @@ func testNewUser(m mocks) *User { m.clientManager.EXPECT().GetClient("user").Return(m.pmapiClient).MinTimes(1) mockConnectedUser(m) - - gomock.InOrder( - m.pmapiClient.EXPECT().GetEvent("").Return(testPMAPIEvent, nil).MaxTimes(1), - m.pmapiClient.EXPECT().GetEvent(testPMAPIEvent.EventID).Return(testPMAPIEvent, nil).MaxTimes(1), - m.pmapiClient.EXPECT().ListMessages(gomock.Any()).Return([]*pmapi.Message{}, 0, nil).MaxTimes(1), - ) + mockEventLoopNoAction(m) user, err := newUser(m.PanicHandler, "user", m.eventListener, m.credentialsStore, m.clientManager, m.storeMaker) assert.NoError(m.t, err) @@ -53,12 +46,7 @@ func testNewUserForLogout(m mocks) *User { m.clientManager.EXPECT().GetClient("user").Return(m.pmapiClient).MinTimes(1) mockConnectedUser(m) - - gomock.InOrder( - m.pmapiClient.EXPECT().GetEvent("").Return(testPMAPIEvent, nil).MaxTimes(1), - m.pmapiClient.EXPECT().GetEvent(testPMAPIEvent.EventID).Return(testPMAPIEvent, nil).MaxTimes(1), - m.pmapiClient.EXPECT().ListMessages(gomock.Any()).Return([]*pmapi.Message{}, 0, nil).MaxTimes(1), - ) + mockEventLoopNoAction(m) user, err := newUser(m.PanicHandler, "user", m.eventListener, m.credentialsStore, m.clientManager, m.storeMaker) assert.NoError(m.t, err) diff --git a/internal/users/users_new_test.go b/internal/users/users_new_test.go index 6b085c8a..c30a130d 100644 --- a/internal/users/users_new_test.go +++ b/internal/users/users_new_test.go @@ -23,7 +23,6 @@ import ( "github.com/ProtonMail/proton-bridge/internal/events" "github.com/ProtonMail/proton-bridge/internal/users/credentials" - "github.com/ProtonMail/proton-bridge/pkg/pmapi" gomock "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" ) @@ -86,36 +85,6 @@ func TestNewUsersWithConnectedUserWithBadToken(t *testing.T) { checkUsersNew(t, m, []*credentials.Credentials{testCredentialsDisconnected}) } -func mockConnectedUser(m mocks) { - gomock.InOrder( - m.credentialsStore.EXPECT().Get("user").Return(testCredentials, nil), - - m.credentialsStore.EXPECT().Get("user").Return(testCredentials, nil), - m.pmapiClient.EXPECT().AuthRefresh("token").Return(testAuthRefresh, nil), - - m.pmapiClient.EXPECT().Unlock([]byte(testCredentials.MailboxPassword)).Return(nil), - - // Set up mocks for store initialisation for the authorized user. - m.pmapiClient.EXPECT().ListLabels().Return([]*pmapi.Label{}, nil), - m.pmapiClient.EXPECT().CountMessages("").Return([]*pmapi.MessagesCount{}, nil), - m.pmapiClient.EXPECT().Addresses().Return([]*pmapi.Address{testPMAPIAddress}), - ) -} - -// mockAuthUpdate simulates users calling UpdateAuthToken on the given user. -// This would normally be done by users when it receives an auth from the ClientManager, -// but as we don't have a full users instance here, we do this manually. -func mockAuthUpdate(user *User, token string, m mocks) { - gomock.InOrder( - m.credentialsStore.EXPECT().UpdateToken("user", ":"+token).Return(nil), - m.credentialsStore.EXPECT().Get("user").Return(credentialsWithToken(token), nil), - ) - - user.updateAuthToken(refreshWithToken(token)) - - waitForEvents() -} - func TestNewUsersWithConnectedUser(t *testing.T) { m := initMocks(t) defer m.ctrl.Finish() diff --git a/internal/users/users_test.go b/internal/users/users_test.go index f79fe162..e5c9cece 100644 --- a/internal/users/users_test.go +++ b/internal/users/users_test.go @@ -284,10 +284,37 @@ func TestClearData(t *testing.T) { func mockEventLoopNoAction(m mocks) { // Set up mocks for starting the store's event loop (in store.New). // The event loop runs in another goroutine so this might happen at any time. + m.pmapiClient.EXPECT().GetEvent("").Return(testPMAPIEvent, nil).AnyTimes() + m.pmapiClient.EXPECT().GetEvent(testPMAPIEvent.EventID).Return(testPMAPIEvent, nil).AnyTimes() + m.pmapiClient.EXPECT().ListMessages(gomock.Any()).Return([]*pmapi.Message{}, 0, nil).AnyTimes() +} + +func mockConnectedUser(m mocks) { gomock.InOrder( - m.pmapiClient.EXPECT().GetEvent("").Return(testPMAPIEvent, nil), - m.pmapiClient.EXPECT().GetEvent(testPMAPIEvent.EventID).Return(testPMAPIEvent, nil), - // Set up mocks for performing the initial store sync. - m.pmapiClient.EXPECT().ListMessages(gomock.Any()).Return([]*pmapi.Message{}, 0, nil), + m.credentialsStore.EXPECT().Get("user").Return(testCredentials, nil), + + m.credentialsStore.EXPECT().Get("user").Return(testCredentials, nil), + m.pmapiClient.EXPECT().AuthRefresh("token").Return(testAuthRefresh, nil), + + m.pmapiClient.EXPECT().Unlock([]byte(testCredentials.MailboxPassword)).Return(nil), + + // Set up mocks for store initialisation for the authorized user. + m.pmapiClient.EXPECT().ListLabels().Return([]*pmapi.Label{}, nil), + m.pmapiClient.EXPECT().CountMessages("").Return([]*pmapi.MessagesCount{}, nil), + m.pmapiClient.EXPECT().Addresses().Return([]*pmapi.Address{testPMAPIAddress}), ) } + +// mockAuthUpdate simulates users calling UpdateAuthToken on the given user. +// This would normally be done by users when it receives an auth from the ClientManager, +// but as we don't have a full users instance here, we do this manually. +func mockAuthUpdate(user *User, token string, m mocks) { + gomock.InOrder( + m.credentialsStore.EXPECT().UpdateToken("user", ":"+token).Return(nil), + m.credentialsStore.EXPECT().Get("user").Return(credentialsWithToken(token), nil), + ) + + user.updateAuthToken(refreshWithToken(token)) + + waitForEvents() +} diff --git a/pkg/pmapi/check_connection_test.go b/pkg/pmapi/check_connection_test.go index 75c9754c..5c1db7cc 100644 --- a/pkg/pmapi/check_connection_test.go +++ b/pkg/pmapi/check_connection_test.go @@ -43,7 +43,7 @@ func startServer() { _, _ = w.Write([]byte("OK")) }) http.HandleFunc("/timeout", func(w http.ResponseWriter, r *http.Request) { - time.Sleep(10 * time.Second) + time.Sleep(testRequestTimeout + time.Second) // Add extra second to be sure it will timeout. w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte("OK")) }) diff --git a/unreleased.md b/unreleased.md index a2189f06..84690755 100644 --- a/unreleased.md +++ b/unreleased.md @@ -15,3 +15,4 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/) ### Fixed * GODT-135 Support parameters in SMTP `FROM MAIL` command, such as `BODY=7BIT`, or empty value `FROM MAIL:<>` used by some clients. +* GODT-338 GODT-781 GODT-857 GODT-866 Flaky tests.