From ceae143e78dabe9c5ef6bafff739aa487f79ca70 Mon Sep 17 00:00:00 2001 From: Ethan Koenig Date: Tue, 7 Feb 2017 06:47:55 -0500 Subject: [PATCH] Consistency checks for unit tests (#853) --- models/consistency_test.go | 149 ++++++++++++++++++++++++++++++++++++ models/models.go | 1 + models/notification_test.go | 1 + models/org_test.go | 16 ++++ models/pull_test.go | 19 ++--- models/setup_for_test.go | 8 +- 6 files changed, 182 insertions(+), 12 deletions(-) create mode 100644 models/consistency_test.go diff --git a/models/consistency_test.go b/models/consistency_test.go new file mode 100644 index 000000000..703d5e383 --- /dev/null +++ b/models/consistency_test.go @@ -0,0 +1,149 @@ +// Copyright 2017 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/assert" +) + +// ConsistencyCheckable a type that can be tested for database consistency +type ConsistencyCheckable interface { + CheckForConsistency(t *testing.T) +} + +// CheckConsistencyForAll test that the entire database is consistent +func CheckConsistencyForAll(t *testing.T) { + CheckConsistencyFor(t, + &User{}, + &Repository{}, + &Issue{}, + &PullRequest{}, + &Milestone{}, + &Label{}, + &Team{}) +} + +// CheckConsistencyFor test that all matching database entries are consistent +func CheckConsistencyFor(t *testing.T, beansToCheck ...interface{}) { + for _, bean := range beansToCheck { + sliceType := reflect.SliceOf(reflect.TypeOf(bean)) + sliceValue := reflect.MakeSlice(sliceType, 0, 10) + + ptrToSliceValue := reflect.New(sliceType) + ptrToSliceValue.Elem().Set(sliceValue) + + assert.NoError(t, x.Find(ptrToSliceValue.Interface())) + sliceValue = ptrToSliceValue.Elem() + + for i := 0; i < sliceValue.Len(); i++ { + entity := sliceValue.Index(i).Interface() + checkable, ok := entity.(ConsistencyCheckable) + if !ok { + t.Errorf("Expected %+v (of type %T) to be checkable for consistency", + entity, entity) + } else { + checkable.CheckForConsistency(t) + } + } + } +} + +// getCount get the count of database entries matching bean +func getCount(t *testing.T, e Engine, bean interface{}) int64 { + count, err := e.Count(bean) + assert.NoError(t, err) + return count +} + +// assertCount test the count of database entries matching bean +func assertCount(t *testing.T, bean interface{}, expected int) { + assert.EqualValues(t, expected, getCount(t, x, bean), + "Failed consistency test, the counted bean (of type %T) was %+v", bean, bean) +} + +func (user *User) CheckForConsistency(t *testing.T) { + assertCount(t, &Repository{OwnerID: user.ID}, user.NumRepos) + assertCount(t, &Star{UID: user.ID}, user.NumStars) + assertCount(t, &OrgUser{OrgID: user.ID}, user.NumMembers) + assertCount(t, &Team{OrgID: user.ID}, user.NumTeams) + assertCount(t, &Follow{UserID: user.ID}, user.NumFollowing) + assertCount(t, &Follow{FollowID: user.ID}, user.NumFollowers) +} + +func (repo *Repository) CheckForConsistency(t *testing.T) { + assertCount(t, &Star{RepoID: repo.ID}, repo.NumStars) + assertCount(t, &Watch{RepoID: repo.ID}, repo.NumWatches) + assertCount(t, &Issue{RepoID: repo.ID}, repo.NumIssues) + assertCount(t, &Milestone{RepoID: repo.ID}, repo.NumMilestones) + assertCount(t, &Repository{ForkID: repo.ID}, repo.NumForks) + if repo.IsFork { + AssertExistsAndLoadBean(t, &Repository{ID: repo.ForkID}) + } + + actual := getCount(t, x.Where("is_closed=1"), &Issue{RepoID: repo.ID}) + assert.EqualValues(t, repo.NumClosedIssues, actual, + "Unexpected number of closed issues for repo %+v", repo) + + actual = getCount(t, x.Where("is_pull=1"), &Issue{RepoID: repo.ID}) + assert.EqualValues(t, repo.NumPulls, actual, + "Unexpected number of pulls for repo %+v", repo) + + actual = getCount(t, x.Where("is_pull=1 AND is_closed=1"), &Issue{RepoID: repo.ID}) + assert.EqualValues(t, repo.NumClosedPulls, actual, + "Unexpected number of closed pulls for repo %+v", repo) + + actual = getCount(t, x.Where("is_closed=1"), &Milestone{RepoID: repo.ID}) + assert.EqualValues(t, repo.NumClosedMilestones, actual, + "Unexpected number of closed milestones for repo %+v", repo) +} + +func (issue *Issue) CheckForConsistency(t *testing.T) { + assertCount(t, &Comment{IssueID: issue.ID}, issue.NumComments) + if issue.IsPull { + pr := AssertExistsAndLoadBean(t, &PullRequest{IssueID: issue.ID}).(*PullRequest) + assert.EqualValues(t, pr.Index, issue.Index) + } +} + +func (pr *PullRequest) CheckForConsistency(t *testing.T) { + issue := AssertExistsAndLoadBean(t, &Issue{ID: pr.IssueID}).(*Issue) + assert.True(t, issue.IsPull) + assert.EqualValues(t, issue.Index, pr.Index) +} + +func (milestone *Milestone) CheckForConsistency(t *testing.T) { + assertCount(t, &Issue{MilestoneID: milestone.ID}, milestone.NumIssues) + + actual := getCount(t, x.Where("is_closed=1"), &Issue{MilestoneID: milestone.ID}) + assert.EqualValues(t, milestone.NumClosedIssues, actual, + "Unexpected number of closed issues for milestone %+v", milestone) +} + +func (label *Label) CheckForConsistency(t *testing.T) { + issueLabels := make([]*IssueLabel, 0, 10) + assert.NoError(t, x.Find(&issueLabels, &IssueLabel{LabelID: label.ID})) + assert.EqualValues(t, label.NumIssues, len(issueLabels), + "Unexpected number of issue for label %+v", label) + + issueIDs := make([]int64, len(issueLabels)) + for i, issueLabel := range issueLabels { + issueIDs[i] = issueLabel.IssueID + } + + expected := int64(0) + if len(issueIDs) > 0 { + expected = getCount(t, x.In("id", issueIDs).Where("is_closed=1"), &Issue{}) + } + assert.EqualValues(t, expected, label.NumClosedIssues, + "Unexpected number of closed issues for label %+v", label) +} + +func (team *Team) CheckForConsistency(t *testing.T) { + assertCount(t, &TeamUser{TeamID: team.ID}, team.NumMembers) + assertCount(t, &TeamRepo{TeamID: team.ID}, team.NumRepos) +} diff --git a/models/models.go b/models/models.go index 1f49640dd..659083403 100644 --- a/models/models.go +++ b/models/models.go @@ -30,6 +30,7 @@ import ( // Engine represents a xorm engine or session. type Engine interface { + Count(interface{}) (int64, error) Decr(column string, arg ...interface{}) *xorm.Session Delete(interface{}) (int64, error) Exec(string, ...interface{}) (sql.Result, error) diff --git a/models/notification_test.go b/models/notification_test.go index dccb478f2..57d686a47 100644 --- a/models/notification_test.go +++ b/models/notification_test.go @@ -20,6 +20,7 @@ func TestCreateOrUpdateIssueNotifications(t *testing.T) { assert.Equal(t, NotificationStatusUnread, notf.Status) notf = AssertExistsAndLoadBean(t, &Notification{UserID: 4, IssueID: issue.ID}).(*Notification) assert.Equal(t, NotificationStatusUnread, notf.Status) + CheckConsistencyFor(t, &Issue{ID: issue.ID}) } func TestNotificationsForUser(t *testing.T) { diff --git a/models/org_test.go b/models/org_test.go index bf505b0ab..95698a58e 100644 --- a/models/org_test.go +++ b/models/org_test.go @@ -101,6 +101,8 @@ func TestUser_AddMember(t *testing.T) { AssertExistsAndLoadBean(t, &OrgUser{UID: 4, OrgID: 3}) org = AssertExistsAndLoadBean(t, &User{ID: 3}).(*User) assert.Equal(t, prevNumMembers, org.NumMembers) + + CheckConsistencyFor(t, &User{}) } func TestUser_RemoveMember(t *testing.T) { @@ -122,6 +124,8 @@ func TestUser_RemoveMember(t *testing.T) { AssertNotExistsBean(t, &OrgUser{UID: 5, OrgID: 3}) org = AssertExistsAndLoadBean(t, &User{ID: 3}).(*User) assert.Equal(t, prevNumMembers, org.NumMembers) + + CheckConsistencyFor(t, &User{}, &Team{}) } func TestUser_RemoveOrgRepo(t *testing.T) { @@ -140,6 +144,11 @@ func TestUser_RemoveOrgRepo(t *testing.T) { AssertNotExistsBean(t, &TeamRepo{RepoID: repo.ID, OrgID: org.ID}) assert.NoError(t, org.RemoveOrgRepo(NonexistentID)) + + CheckConsistencyFor(t, + &User{ID: org.ID}, + &Team{OrgID: org.ID}, + &Repository{ID: repo.ID}) } func TestCreateOrganization(t *testing.T) { @@ -159,6 +168,7 @@ func TestCreateOrganization(t *testing.T) { ownerTeam := AssertExistsAndLoadBean(t, &Team{Name: ownerTeamName, OrgID: org.ID}).(*Team) AssertExistsAndLoadBean(t, &TeamUser{UID: owner.ID, TeamID: ownerTeam.ID}) + CheckConsistencyFor(t, &User{}, &Team{}) } func TestCreateOrganization2(t *testing.T) { @@ -176,6 +186,7 @@ func TestCreateOrganization2(t *testing.T) { assert.Error(t, err) assert.True(t, IsErrUserNotAllowedCreateOrg(err)) AssertNotExistsBean(t, &User{Name: newOrgName, Type: UserTypeOrganization}) + CheckConsistencyFor(t, &User{}, &Team{}) } func TestCreateOrganization3(t *testing.T) { @@ -188,6 +199,7 @@ func TestCreateOrganization3(t *testing.T) { err := CreateOrganization(org, owner) assert.Error(t, err) assert.True(t, IsErrUserAlreadyExist(err)) + CheckConsistencyFor(t, &User{}, &Team{}) } func TestCreateOrganization4(t *testing.T) { @@ -198,6 +210,7 @@ func TestCreateOrganization4(t *testing.T) { err := CreateOrganization(&User{Name: "assets"}, owner) assert.Error(t, err) assert.True(t, IsErrNameReserved(err)) + CheckConsistencyFor(t, &User{}, &Team{}) } func TestGetOrgByName(t *testing.T) { @@ -257,6 +270,7 @@ func TestDeleteOrganization(t *testing.T) { nonOrg := AssertExistsAndLoadBean(t, &User{ID: 5}).(*User) assert.Error(t, DeleteOrganization(nonOrg)) + CheckConsistencyFor(t, &User{}, &Team{}) } func TestIsOrganizationOwner(t *testing.T) { @@ -416,6 +430,7 @@ func TestAddOrgUser(t *testing.T) { testSuccess(3, 5) testSuccess(3, 5) testSuccess(6, 2) + CheckConsistencyFor(t, &User{}, &Team{}) } func TestRemoveOrgUser(t *testing.T) { @@ -438,6 +453,7 @@ func TestRemoveOrgUser(t *testing.T) { assert.Error(t, err) assert.True(t, IsErrLastOrgOwner(err)) AssertExistsAndLoadBean(t, &OrgUser{OrgID: 7, UID: 5}) + CheckConsistencyFor(t, &User{}, &Team{}) } func TestUser_GetUserTeamIDs(t *testing.T) { diff --git a/models/pull_test.go b/models/pull_test.go index b8794e131..86c71f565 100644 --- a/models/pull_test.go +++ b/models/pull_test.go @@ -155,34 +155,30 @@ func TestGetPullRequestByIssueID(t *testing.T) { func TestPullRequest_Update(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) - pr := &PullRequest{ - ID: 1, - IssueID: 100, - BaseBranch: "baseBranch", - HeadBranch: "headBranch", - } + pr := AssertExistsAndLoadBean(t, &PullRequest{ID: 1}).(*PullRequest) + pr.BaseBranch = "baseBranch" + pr.HeadBranch = "headBranch" pr.Update() - pr = AssertExistsAndLoadBean(t, &PullRequest{ID: 1}).(*PullRequest) - assert.Equal(t, int64(100), pr.IssueID) + pr = AssertExistsAndLoadBean(t, &PullRequest{ID: pr.ID}).(*PullRequest) assert.Equal(t, "baseBranch", pr.BaseBranch) assert.Equal(t, "headBranch", pr.HeadBranch) + CheckConsistencyFor(t, pr) } func TestPullRequest_UpdateCols(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) pr := &PullRequest{ ID: 1, - IssueID: int64(100), BaseBranch: "baseBranch", HeadBranch: "headBranch", } - pr.UpdateCols("issue_id", "head_branch") + pr.UpdateCols("head_branch") pr = AssertExistsAndLoadBean(t, &PullRequest{ID: 1}).(*PullRequest) - assert.Equal(t, int64(100), pr.IssueID) assert.Equal(t, "master", pr.BaseBranch) assert.Equal(t, "headBranch", pr.HeadBranch) + CheckConsistencyFor(t, pr) } // TODO TestPullRequest_UpdatePatch @@ -232,4 +228,5 @@ func TestChangeUsernameInPullRequests(t *testing.T) { for _, pr := range prs { assert.Equal(t, newUsername, pr.HeadUserName) } + CheckConsistencyFor(t, &PullRequest{}) } diff --git a/models/setup_for_test.go b/models/setup_for_test.go index 651e36e82..91d5442c8 100644 --- a/models/setup_for_test.go +++ b/models/setup_for_test.go @@ -56,6 +56,11 @@ func CreateTestEngine() error { return err } +func TestFixturesAreConsistent(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + CheckConsistencyForAll(t) +} + // PrepareTestDatabase load test fixtures into test database func PrepareTestDatabase() error { return fixtures.Load() @@ -84,7 +89,8 @@ func AssertExistsAndLoadBean(t *testing.T, bean interface{}, conditions ...inter exists, err := loadBeanIfExists(bean, conditions...) assert.NoError(t, err) assert.True(t, exists, - "Expected to find %+v (with conditions %+v), but did not", bean, conditions) + "Expected to find %+v (of type %T, with conditions %+v), but did not", + bean, bean, conditions) return bean }