From a7086e5e4918bd60e622959544bbe6561e9df936 Mon Sep 17 00:00:00 2001 From: kolaente Date: Wed, 25 Feb 2026 09:42:32 +0100 Subject: [PATCH] fix: prevent session leaks and visibility issues in model tests Two categories of fixes: 1. Use defer s.Close() instead of explicit s.Close() to prevent session leaks when require.FailNow() triggers runtime.Goexit(), which skips explicit close calls but runs deferred functions. Leaked sessions hold SQLite write locks that block all subsequent fixture loading. 2. Add s.Commit() before db.AssertExists/db.AssertMissing calls. These assertion helpers query via the global engine (not the test session), so they cannot see uncommitted data from the session's transaction. For block-scoped sessions (kanban_task_bucket_test.go), wrap each block in an anonymous function so defer runs at block boundary rather than deferring to the enclosing test function. --- pkg/models/kanban_task_bucket_test.go | 30 +++++++++--------- pkg/models/kanban_test.go | 3 ++ pkg/models/label_task_test.go | 12 ++++--- pkg/models/label_test.go | 15 +++++---- pkg/models/link_sharing_test.go | 2 ++ pkg/models/mentions_test.go | 1 + ...project_permissions_multiple_teams_test.go | 4 +-- pkg/models/project_team_test.go | 23 ++++++++------ pkg/models/project_test.go | 31 ++++++++++++------- pkg/models/project_users_permissions_test.go | 2 +- pkg/models/project_users_test.go | 5 ++- pkg/models/reaction_test.go | 1 + pkg/models/subscription_test.go | 2 ++ pkg/models/task_attachment_test.go | 4 +++ pkg/models/task_comments_test.go | 2 ++ pkg/models/tasks_test.go | 1 + pkg/models/user_delete_test.go | 3 ++ 17 files changed, 89 insertions(+), 52 deletions(-) diff --git a/pkg/models/kanban_task_bucket_test.go b/pkg/models/kanban_task_bucket_test.go index 968e338fd..a4d791370 100644 --- a/pkg/models/kanban_task_bucket_test.go +++ b/pkg/models/kanban_task_bucket_test.go @@ -183,57 +183,57 @@ func TestTaskBucket_Update(t *testing.T) { doneAt := time.Now().Round(time.Second) // Set a done timestamp on the task - { + func() { s := db.NewSession() + defer s.Close() _, err := s.ID(2).Cols("done_at").Update(&Task{DoneAt: doneAt}) require.NoError(t, err) err = s.Commit() require.NoError(t, err) - s.Close() - } + }() // Move the task to another project without a done bucket using Task.Update - { + func() { s := db.NewSession() + defer s.Close() task := &Task{ID: 2, Done: true, ProjectID: 9} err := task.Update(s, u) require.NoError(t, err) err = s.Commit() require.NoError(t, err) - s.Close() - } + }() // Verify the task still has the same done timestamp - { + func() { s := db.NewSession() + defer s.Close() var task Task _, err := s.ID(2).Get(&task) require.NoError(t, err) assert.True(t, task.Done) assert.WithinDuration(t, doneAt, task.DoneAt, time.Second) - s.Close() - } + }() // Move the task back to the original project with a done bucket using Task.Update - { + func() { s := db.NewSession() + defer s.Close() task := &Task{ID: 2, Done: true, ProjectID: 1} err := task.Update(s, u) require.NoError(t, err) err = s.Commit() require.NoError(t, err) - s.Close() - } + }() // Verify the done timestamp is still preserved - { + func() { s := db.NewSession() + defer s.Close() var task Task _, err := s.ID(2).Get(&task) require.NoError(t, err) assert.True(t, task.Done) assert.WithinDuration(t, doneAt, task.DoneAt, time.Second) - s.Close() - } + }() }) } diff --git a/pkg/models/kanban_test.go b/pkg/models/kanban_test.go index 9cf610ad9..99fdbca34 100644 --- a/pkg/models/kanban_test.go +++ b/pkg/models/kanban_test.go @@ -216,6 +216,9 @@ func TestBucket_Delete(t *testing.T) { err := b.Delete(s, u) require.NoError(t, err) + err = s.Commit() + require.NoError(t, err) + db.AssertExists(t, "project_views", map[string]interface{}{ "id": b.ProjectViewID, "done_bucket_id": 0, diff --git a/pkg/models/label_task_test.go b/pkg/models/label_task_test.go index ceccc1931..5567b9e9e 100644 --- a/pkg/models/label_task_test.go +++ b/pkg/models/label_task_test.go @@ -24,9 +24,10 @@ import ( "code.vikunja.io/api/pkg/db" "code.vikunja.io/api/pkg/user" - "gopkg.in/d4l3k/messagediff.v1" - "code.vikunja.io/api/pkg/web" + + "github.com/stretchr/testify/require" + "gopkg.in/d4l3k/messagediff.v1" ) func TestLabelTask_ReadAll(t *testing.T) { @@ -129,6 +130,7 @@ func TestLabelTask_ReadAll(t *testing.T) { t.Run(tt.name, func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() l := &LabelTask{ ID: tt.fields.ID, @@ -149,8 +151,6 @@ func TestLabelTask_ReadAll(t *testing.T) { if diff, equal := messagediff.PrettyDiff(gotLabels, tt.wantLabels); !equal { t.Errorf("LabelTask.ReadAll() = %v, want %v, diff: %v", l, tt.wantLabels, diff) } - - s.Close() }) } } @@ -227,6 +227,7 @@ func TestLabelTask_Create(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() l := &LabelTask{ ID: tt.fields.ID, @@ -248,13 +249,13 @@ func TestLabelTask_Create(t *testing.T) { t.Errorf("LabelTask.Create() Wrong error type! Error = %v, want = %v", err, runtime.FuncForPC(reflect.ValueOf(tt.errType).Pointer()).Name()) } if !tt.wantErr { + require.NoError(t, s.Commit()) db.AssertExists(t, "label_tasks", map[string]interface{}{ "id": l.ID, "task_id": l.TaskID, "label_id": l.LabelID, }, false) } - s.Close() }) } } @@ -348,6 +349,7 @@ func TestLabelTask_Delete(t *testing.T) { if (err != nil) && tt.wantErr && !tt.errType(err) { t.Errorf("LabelTask.Delete() Wrong error type! Error = %v, want = %v", err, runtime.FuncForPC(reflect.ValueOf(tt.errType).Pointer()).Name()) } + require.NoError(t, s.Commit()) db.AssertMissing(t, "label_tasks", map[string]interface{}{ "label_id": l.LabelID, "task_id": l.TaskID, diff --git a/pkg/models/label_test.go b/pkg/models/label_test.go index 04dbfba38..3fece8e7c 100644 --- a/pkg/models/label_test.go +++ b/pkg/models/label_test.go @@ -26,6 +26,7 @@ import ( "code.vikunja.io/api/pkg/user" "code.vikunja.io/api/pkg/web" + "github.com/stretchr/testify/require" "gopkg.in/d4l3k/messagediff.v1" ) @@ -150,6 +151,7 @@ func TestLabel_ReadAll(t *testing.T) { } db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() gotLs, _, _, err := l.ReadAll(s, tt.args.a, tt.args.search, tt.args.page, 0) if (err != nil) != tt.wantErr { t.Errorf("Label.ReadAll() error = %v, wantErr %v", err, tt.wantErr) @@ -161,7 +163,6 @@ func TestLabel_ReadAll(t *testing.T) { if diff, equal := messagediff.PrettyDiff(got, tt.wantLs); !equal { t.Errorf("Label.ReadAll() = %v, want %v, diff: %v", gotLs, tt.wantLs, diff) } - s.Close() }) } } @@ -276,6 +277,7 @@ func TestLabel_ReadOne(t *testing.T) { } s := db.NewSession() + defer s.Close() allowed, _, _ := l.CanRead(s, tt.auth) if !allowed && !tt.wantForbidden { @@ -291,8 +293,6 @@ func TestLabel_ReadOne(t *testing.T) { if diff, equal := messagediff.PrettyDiff(l, tt.want); !equal && !tt.wantErr && !tt.wantForbidden { t.Errorf("Label.ReadAll() = %v, want %v, diff: %v", l, tt.want, diff) } - - s.Close() }) } } @@ -347,6 +347,7 @@ func TestLabel_Create(t *testing.T) { Permissions: tt.fields.Permissions, } s := db.NewSession() + defer s.Close() allowed, _ := l.CanCreate(s, tt.args.a) if !allowed && !tt.wantForbidden { t.Errorf("Label.CanCreate() forbidden, want %v", tt.wantForbidden) @@ -355,6 +356,7 @@ func TestLabel_Create(t *testing.T) { t.Errorf("Label.Create() error = %v, wantErr %v", err, tt.wantErr) } if !tt.wantErr { + require.NoError(t, s.Commit()) db.AssertExists(t, "labels", map[string]interface{}{ "id": l.ID, "title": l.Title, @@ -362,7 +364,6 @@ func TestLabel_Create(t *testing.T) { "hex_color": l.HexColor, }, false) } - _ = s.Close() }) } } @@ -439,6 +440,7 @@ func TestLabel_Update(t *testing.T) { Permissions: tt.fields.Permissions, } s := db.NewSession() + defer s.Close() allowed, _ := l.CanUpdate(s, tt.auth) if !allowed && !tt.wantForbidden { t.Errorf("Label.CanUpdate() forbidden, want %v", tt.wantForbidden) @@ -447,12 +449,12 @@ func TestLabel_Update(t *testing.T) { t.Errorf("Label.Update() error = %v, wantErr %v", err, tt.wantErr) } if !tt.wantErr && !tt.wantForbidden { + require.NoError(t, s.Commit()) db.AssertExists(t, "labels", map[string]interface{}{ "id": tt.fields.ID, "title": tt.fields.Title, }, false) } - _ = s.Close() }) } } @@ -525,6 +527,7 @@ func TestLabel_Delete(t *testing.T) { Permissions: tt.fields.Permissions, } s := db.NewSession() + defer s.Close() allowed, _ := l.CanDelete(s, tt.auth) if !allowed && !tt.wantForbidden { t.Errorf("Label.CanDelete() forbidden, want %v", tt.wantForbidden) @@ -533,11 +536,11 @@ func TestLabel_Delete(t *testing.T) { t.Errorf("Label.Delete() error = %v, wantErr %v", err, tt.wantErr) } if !tt.wantErr && !tt.wantForbidden { + require.NoError(t, s.Commit()) db.AssertMissing(t, "labels", map[string]interface{}{ "id": l.ID, }) } - _ = s.Close() }) } } diff --git a/pkg/models/link_sharing_test.go b/pkg/models/link_sharing_test.go index 6e85da88c..97748feef 100644 --- a/pkg/models/link_sharing_test.go +++ b/pkg/models/link_sharing_test.go @@ -45,6 +45,7 @@ func TestLinkSharing_Create(t *testing.T) { assert.NotEmpty(t, share.Hash) assert.NotEmpty(t, share.ID) assert.Equal(t, SharingTypeWithoutPassword, share.SharingType) + require.NoError(t, s.Commit()) db.AssertExists(t, "link_shares", map[string]interface{}{ "id": share.ID, }, false) @@ -79,6 +80,7 @@ func TestLinkSharing_Create(t *testing.T) { assert.NotEmpty(t, share.Hash) assert.NotEmpty(t, share.ID) assert.Empty(t, share.Password) + require.NoError(t, s.Commit()) db.AssertExists(t, "link_shares", map[string]interface{}{ "id": share.ID, "sharing_type": SharingTypeWithPassword, diff --git a/pkg/models/mentions_test.go b/pkg/models/mentions_test.go index 58b01794f..a4a085482 100644 --- a/pkg/models/mentions_test.go +++ b/pkg/models/mentions_test.go @@ -119,6 +119,7 @@ func TestSendingMentionNotification(t *testing.T) { _, err = notifyMentionedUsers(s, &task, tc.Comment, n) require.NoError(t, err) + require.NoError(t, s.Commit()) db.AssertExists(t, "notifications", map[string]interface{}{ "subject_id": tc.ID, "notifiable_id": 1, diff --git a/pkg/models/project_permissions_multiple_teams_test.go b/pkg/models/project_permissions_multiple_teams_test.go index 04efaaeeb..46e47b23b 100644 --- a/pkg/models/project_permissions_multiple_teams_test.go +++ b/pkg/models/project_permissions_multiple_teams_test.go @@ -97,8 +97,8 @@ func TestProjectPermissions_OwnerInMultipleTeamsWithDifferentPermissions(t *test // Setup sharing with Team Y s = db.NewSession() + defer s.Close() prepareSharingY(s) - s.Close() // Verify User A still has admin permissions after sharing with Team Y t.Run("owner has admin permissions after sharing with team Y (admin)", func(t *testing.T) { @@ -142,8 +142,8 @@ func TestProjectPermissions_OwnerInMultipleTeamsWithDifferentPermissions(t *test // Setup sharing with Team Z s = db.NewSession() + defer s.Close() prepareTeamSharedMultiple(s) - s.Close() // Verify User A STILL has admin permissions after sharing with Team Z // user should retain highest permission (owner/admin), not be downgraded to read-only diff --git a/pkg/models/project_team_test.go b/pkg/models/project_team_test.go index e907c9966..a759dc3a5 100644 --- a/pkg/models/project_team_test.go +++ b/pkg/models/project_team_test.go @@ -40,12 +40,12 @@ func TestTeamProject_ReadAll(t *testing.T) { } db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() teams, _, _, err := tl.ReadAll(s, u, "", 1, 50) require.NoError(t, err) assert.Equal(t, reflect.Slice, reflect.TypeOf(teams).Kind()) ts := reflect.ValueOf(teams) assert.Equal(t, 1, ts.Len()) - _ = s.Close() }) t.Run("nonexistant project", func(t *testing.T) { tl := TeamProject{ @@ -53,10 +53,10 @@ func TestTeamProject_ReadAll(t *testing.T) { } db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() _, _, _, err := tl.ReadAll(s, u, "", 1, 50) require.Error(t, err) assert.True(t, IsErrProjectDoesNotExist(err)) - _ = s.Close() }) t.Run("no access", func(t *testing.T) { tl := TeamProject{ @@ -66,10 +66,10 @@ func TestTeamProject_ReadAll(t *testing.T) { } db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() _, _, _, err := tl.ReadAll(s, u, "", 1, 50) require.Error(t, err) assert.True(t, IsErrNeedToHaveProjectReadAccess(err)) - _ = s.Close() }) t.Run("search", func(t *testing.T) { tl := TeamProject{ @@ -77,13 +77,13 @@ func TestTeamProject_ReadAll(t *testing.T) { } db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() teams, _, _, err := tl.ReadAll(s, u, "TEAM9", 1, 50) require.NoError(t, err) assert.Equal(t, reflect.Slice, reflect.TypeOf(teams).Kind()) ts := teams.([]*TeamWithPermission) assert.Len(t, ts, 1) assert.Equal(t, int64(9), ts[0].ID) - _ = s.Close() }) } @@ -92,6 +92,7 @@ func TestTeamProject_Create(t *testing.T) { t.Run("normal", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() tl := TeamProject{ TeamID: 1, ProjectID: 1, @@ -112,6 +113,7 @@ func TestTeamProject_Create(t *testing.T) { t.Run("team already has access", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() tl := TeamProject{ TeamID: 1, ProjectID: 3, @@ -120,11 +122,11 @@ func TestTeamProject_Create(t *testing.T) { err := tl.Create(s, u) require.Error(t, err) assert.True(t, IsErrTeamAlreadyHasAccess(err)) - _ = s.Close() }) t.Run("wrong permissions", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() tl := TeamProject{ TeamID: 1, ProjectID: 1, @@ -133,11 +135,11 @@ func TestTeamProject_Create(t *testing.T) { err := tl.Create(s, u) require.Error(t, err) assert.True(t, IsErrInvalidPermission(err)) - _ = s.Close() }) t.Run("nonexistant team", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() tl := TeamProject{ TeamID: 9999, ProjectID: 1, @@ -145,11 +147,11 @@ func TestTeamProject_Create(t *testing.T) { err := tl.Create(s, u) require.Error(t, err) assert.True(t, IsErrTeamDoesNotExist(err)) - _ = s.Close() }) t.Run("nonexistant project", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() tl := TeamProject{ TeamID: 1, ProjectID: 9999, @@ -157,7 +159,6 @@ func TestTeamProject_Create(t *testing.T) { err := tl.Create(s, u) require.Error(t, err) assert.True(t, IsErrProjectDoesNotExist(err)) - _ = s.Close() }) } @@ -167,6 +168,7 @@ func TestTeamProject_Delete(t *testing.T) { t.Run("normal", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() tl := TeamProject{ TeamID: 1, ProjectID: 3, @@ -183,6 +185,7 @@ func TestTeamProject_Delete(t *testing.T) { t.Run("nonexistant team", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() tl := TeamProject{ TeamID: 9999, ProjectID: 1, @@ -190,11 +193,11 @@ func TestTeamProject_Delete(t *testing.T) { err := tl.Delete(s, user) require.Error(t, err) assert.True(t, IsErrTeamDoesNotExist(err)) - _ = s.Close() }) t.Run("nonexistant project", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() tl := TeamProject{ TeamID: 1, ProjectID: 9999, @@ -202,7 +205,6 @@ func TestTeamProject_Delete(t *testing.T) { err := tl.Delete(s, user) require.Error(t, err) assert.True(t, IsErrTeamDoesNotHaveAccessToProject(err)) - _ = s.Close() }) } @@ -262,6 +264,7 @@ func TestTeamProject_Update(t *testing.T) { t.Run(tt.name, func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() tl := &TeamProject{ ID: tt.fields.ID, diff --git a/pkg/models/project_test.go b/pkg/models/project_test.go index d0590c1ca..99431c6d4 100644 --- a/pkg/models/project_test.go +++ b/pkg/models/project_test.go @@ -39,6 +39,7 @@ func TestProject_CreateOrUpdate(t *testing.T) { t.Run("normal", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() project := Project{ Title: "test", Description: "Lorem Ipsum", @@ -81,6 +82,7 @@ func TestProject_CreateOrUpdate(t *testing.T) { t.Run("kanban view creates To-Do, doing, done buckets", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() project := Project{ Title: "test kanban buckets", Description: "Lorem Ipsum", @@ -119,6 +121,7 @@ func TestProject_CreateOrUpdate(t *testing.T) { t.Run("nonexistent parent", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() project := Project{ Title: "test", Description: "Lorem Ipsum", @@ -127,11 +130,11 @@ func TestProject_CreateOrUpdate(t *testing.T) { err := project.Create(s, usr) require.Error(t, err) assert.True(t, IsErrProjectDoesNotExist(err)) - _ = s.Close() }) t.Run("nonexistent owner", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() usr := &user.User{ID: 9482385} project := Project{ Title: "test", @@ -140,11 +143,11 @@ func TestProject_CreateOrUpdate(t *testing.T) { err := project.Create(s, usr) require.Error(t, err) assert.True(t, user.IsErrUserDoesNotExist(err)) - _ = s.Close() }) t.Run("existing identifier", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() project := Project{ Title: "test", Description: "Lorem Ipsum", @@ -153,11 +156,11 @@ func TestProject_CreateOrUpdate(t *testing.T) { err := project.Create(s, usr) require.Error(t, err) assert.True(t, IsErrProjectIdentifierIsNotUnique(err)) - _ = s.Close() }) t.Run("non ascii characters", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() project := Project{ Title: "приффки фсем", Description: "Lorem Ipsum", @@ -178,6 +181,7 @@ func TestProject_CreateOrUpdate(t *testing.T) { t.Run("normal", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() project := Project{ ID: 1, Title: "test", @@ -197,6 +201,7 @@ func TestProject_CreateOrUpdate(t *testing.T) { t.Run("nonexistent", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() project := Project{ ID: 99999999, Title: "test", @@ -204,12 +209,11 @@ func TestProject_CreateOrUpdate(t *testing.T) { err := project.Update(s, usr) require.Error(t, err) assert.True(t, IsErrProjectDoesNotExist(err)) - _ = s.Close() - }) t.Run("existing identifier", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() project := Project{ Title: "test", Description: "Lorem Ipsum", @@ -218,7 +222,6 @@ func TestProject_CreateOrUpdate(t *testing.T) { err := project.Create(s, usr) require.Error(t, err) assert.True(t, IsErrProjectIdentifierIsNotUnique(err)) - _ = s.Close() }) t.Run("change parent project", func(t *testing.T) { t.Run("own", func(t *testing.T) { @@ -230,6 +233,7 @@ func TestProject_CreateOrUpdate(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() project := Project{ ID: 6, Title: "Test6", @@ -253,6 +257,7 @@ func TestProject_CreateOrUpdate(t *testing.T) { t.Run("others", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() project := Project{ ID: 1, Title: "Test1", @@ -261,7 +266,6 @@ func TestProject_CreateOrUpdate(t *testing.T) { } can, _ := project.CanUpdate(s, usr) assert.False(t, can) // project is not writeable by us - _ = s.Close() }) t.Run("pseudo project", func(t *testing.T) { usr := &user.User{ @@ -272,6 +276,7 @@ func TestProject_CreateOrUpdate(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() project := Project{ ID: 6, Title: "Test6", @@ -286,6 +291,7 @@ func TestProject_CreateOrUpdate(t *testing.T) { t.Run("archive default project of the same user", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() project := Project{ ID: 4, IsArchived: true, @@ -297,6 +303,7 @@ func TestProject_CreateOrUpdate(t *testing.T) { t.Run("archive default project of another user", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() project := Project{ ID: 4, IsArchived: true, @@ -481,14 +488,15 @@ func TestProject_ReadAll(t *testing.T) { t.Run("all", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() projects, _, err := getAllProjectsForUser(s, 6, &projectOptions{}) require.NoError(t, err) assert.Len(t, projects, 25) - _ = s.Close() }) t.Run("all projects for user", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() u := &user.User{ID: 1} project := Project{} projects3, _, _, err := project.ReadAll(s, u, "", 1, 50) @@ -502,21 +510,21 @@ func TestProject_ReadAll(t *testing.T) { assert.Equal(t, int64(6), ls[2].ID) assert.Equal(t, int64(-1), ls[26].ID) assert.Equal(t, int64(-2), ls[27].ID) - _ = s.Close() }) t.Run("projects for nonexistent user", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() usr := &user.User{ID: 999999} project := Project{} _, _, _, err := project.ReadAll(s, usr, "", 1, 50) require.Error(t, err) assert.True(t, user.IsErrUserDoesNotExist(err)) - _ = s.Close() }) t.Run("search", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() u := &user.User{ID: 1} project := Project{} projects3, _, _, err := project.ReadAll(s, u, "TEST10", 1, 50) @@ -526,11 +534,11 @@ func TestProject_ReadAll(t *testing.T) { require.Len(t, ls, 2) assert.Equal(t, int64(10), ls[0].ID) assert.Equal(t, int64(-1), ls[1].ID) - _ = s.Close() }) t.Run("search returns filters as well", func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() u := &user.User{ID: 1} project := Project{} projects3, _, _, err := project.ReadAll(s, u, "testfilter", 1, 50) @@ -540,7 +548,6 @@ func TestProject_ReadAll(t *testing.T) { require.Len(t, ls, 2) assert.Equal(t, int64(-1), ls[0].ID) assert.Equal(t, int64(-2), ls[1].ID) - _ = s.Close() }) } diff --git a/pkg/models/project_users_permissions_test.go b/pkg/models/project_users_permissions_test.go index 75b240d42..dd9235b5d 100644 --- a/pkg/models/project_users_permissions_test.go +++ b/pkg/models/project_users_permissions_test.go @@ -81,6 +81,7 @@ func TestProjectUser_CanDoSomething(t *testing.T) { t.Run(tt.name, func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() lu := &ProjectUser{ ID: tt.fields.ID, @@ -101,7 +102,6 @@ func TestProjectUser_CanDoSomething(t *testing.T) { if got, _ := lu.CanUpdate(s, tt.args.a); got != tt.want["CanUpdate"] { t.Errorf("ProjectUser.CanUpdate() = %v, want %v", got, tt.want["CanUpdate"]) } - _ = s.Close() }) } } diff --git a/pkg/models/project_users_test.go b/pkg/models/project_users_test.go index 38cb7d20a..10a71c8c5 100644 --- a/pkg/models/project_users_test.go +++ b/pkg/models/project_users_test.go @@ -110,6 +110,7 @@ func TestProjectUser_Create(t *testing.T) { t.Run(tt.name, func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() ul := &ProjectUser{ ID: tt.fields.ID, @@ -240,6 +241,7 @@ func TestProjectUser_ReadAll(t *testing.T) { t.Run(tt.name, func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() ul := &ProjectUser{ ID: tt.fields.ID, @@ -261,7 +263,6 @@ func TestProjectUser_ReadAll(t *testing.T) { if diff, equal := messagediff.PrettyDiff(got, tt.want); !equal { t.Errorf("ProjectUser.ReadAll() = %v, want %v, diff: %v", got, tt.want, diff) } - _ = s.Close() }) } } @@ -322,6 +323,7 @@ func TestProjectUser_Update(t *testing.T) { t.Run(tt.name, func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() lu := &ProjectUser{ ID: tt.fields.ID, @@ -404,6 +406,7 @@ func TestProjectUser_Delete(t *testing.T) { t.Run(tt.name, func(t *testing.T) { db.LoadAndAssertFixtures(t) s := db.NewSession() + defer s.Close() lu := &ProjectUser{ ID: tt.fields.ID, diff --git a/pkg/models/reaction_test.go b/pkg/models/reaction_test.go index 3d497ae7c..3acec4d7b 100644 --- a/pkg/models/reaction_test.go +++ b/pkg/models/reaction_test.go @@ -208,6 +208,7 @@ func TestReaction_Delete(t *testing.T) { err = r.Delete(s, u) require.NoError(t, err) + require.NoError(t, s.Commit()) db.AssertMissing(t, "reactions", map[string]interface{}{ "entity_id": r.EntityID, "entity_kind": ReactionKindTask, diff --git a/pkg/models/subscription_test.go b/pkg/models/subscription_test.go index 2aeab0d7d..067717140 100644 --- a/pkg/models/subscription_test.go +++ b/pkg/models/subscription_test.go @@ -61,6 +61,7 @@ func TestSubscription_Create(t *testing.T) { err = sb.Create(s, u) require.NoError(t, err) + require.NoError(t, s.Commit()) db.AssertExists(t, "subscriptions", map[string]interface{}{ "entity_type": 3, "entity_id": 1, @@ -208,6 +209,7 @@ func TestSubscription_Delete(t *testing.T) { err = sb.Delete(s, u) require.NoError(t, err) + require.NoError(t, s.Commit()) db.AssertMissing(t, "subscriptions", map[string]interface{}{ "entity_type": 3, "entity_id": 2, diff --git a/pkg/models/task_attachment_test.go b/pkg/models/task_attachment_test.go index 258c136d4..c2cc7dcc2 100644 --- a/pkg/models/task_attachment_test.go +++ b/pkg/models/task_attachment_test.go @@ -106,6 +106,10 @@ func TestTaskAttachment_NewAttachment(t *testing.T) { assert.False(t, os.IsNotExist(err)) assert.Equal(t, testuser.ID, ta.CreatedByID) + // Commit so that LoadFileMetaByID (which reads via the global engine) can see the data + err = s.Commit() + require.NoError(t, err) + // Check the file was inserted correctly ta.File = &files.File{ID: ta.FileID} err = ta.File.LoadFileMetaByID() diff --git a/pkg/models/task_comments_test.go b/pkg/models/task_comments_test.go index 2a23fa0ec..b5fe58ad0 100644 --- a/pkg/models/task_comments_test.go +++ b/pkg/models/task_comments_test.go @@ -80,6 +80,7 @@ func TestTaskComment_Create(t *testing.T) { } err = tc.Create(s, u) require.NoError(t, err) + require.NoError(t, s.Commit()) ev := &TaskCommentCreatedEvent{ Task: &task, Doer: u, @@ -107,6 +108,7 @@ func TestTaskComment_Create(t *testing.T) { } err = tc.Create(s, u) require.NoError(t, err) + require.NoError(t, s.Commit()) ev := &TaskCommentCreatedEvent{ Task: &task, diff --git a/pkg/models/tasks_test.go b/pkg/models/tasks_test.go index 85aa6fc1d..c426f6507 100644 --- a/pkg/models/tasks_test.go +++ b/pkg/models/tasks_test.go @@ -169,6 +169,7 @@ func TestTask_Create(t *testing.T) { } err := task.Create(s, usr) require.NoError(t, err) + require.NoError(t, s.Commit()) db.AssertExists(t, "task_buckets", map[string]interface{}{ "task_id": task.ID, "bucket_id": 22, // default bucket of project 6 but with a position of 2 diff --git a/pkg/models/user_delete_test.go b/pkg/models/user_delete_test.go index c9e7cfb3a..852fdcb87 100644 --- a/pkg/models/user_delete_test.go +++ b/pkg/models/user_delete_test.go @@ -37,6 +37,7 @@ func TestDeleteUser(t *testing.T) { err := DeleteUser(s, u) require.NoError(t, err) + require.NoError(t, s.Commit()) db.AssertMissing(t, "users", map[string]interface{}{"id": u.ID}) db.AssertMissing(t, "projects", map[string]interface{}{"id": 24}) // only user6 had access to this project db.AssertExists(t, "projects", map[string]interface{}{"id": 6}, false) @@ -68,6 +69,7 @@ func TestDeleteUser(t *testing.T) { err := DeleteUser(s, u) require.NoError(t, err) + require.NoError(t, s.Commit()) db.AssertMissing(t, "users", map[string]interface{}{"id": u.ID}) db.AssertMissing(t, "projects", map[string]interface{}{"id": 37}) // only user16 had access to this project, and it was their default }) @@ -101,6 +103,7 @@ func TestDeleteUser(t *testing.T) { err = DeleteUser(s, &user.User{ID: 4}) require.NoError(t, err) + require.NoError(t, s.Commit()) db.AssertMissing(t, "task_assignees", map[string]interface{}{"user_id": 4}) db.AssertMissing(t, "subscriptions", map[string]interface{}{"user_id": 4}) db.AssertMissing(t, "team_members", map[string]interface{}{"user_id": 4})