Skip to content

Commit

Permalink
Refactored test
Browse files Browse the repository at this point in the history
- Changed env.go so that the PopulateDB function only populates the DB and added PopulateDBAndPermissions to populate the DB and OpenFGA permissions. This clarifies when a test needs only database objects versus when a test needs database objects and permissions
  • Loading branch information
kian99 committed Nov 2, 2023
1 parent 35cc273 commit 8d0dc1f
Show file tree
Hide file tree
Showing 14 changed files with 233 additions and 235 deletions.
8 changes: 4 additions & 4 deletions internal/db/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ controllers:
region: test-region
priority: 1
`)
env.PopulateDB(c, *s.Database, nil)
env.PopulateDB(c, *s.Database)

cr, err := s.Database.FindRegion(ctx, "testp", "test-region")
c.Assert(err, qt.IsNil)
Expand Down Expand Up @@ -348,15 +348,15 @@ func (s *dbSuite) TestUpdateUserCloudAccess(c *qt.C) {
c.Assert(err, qt.Equals, nil)

env := jimmtest.ParseEnvironment(c, testUpdateUserCloudAccessEnv)
env.PopulateDB(c, *s.Database, nil)
env.PopulateDB(c, *s.Database)

cld := dbmodel.Cloud{
Name: "test-hosted",
}
err = s.Database.GetCloud(ctx, &cld)
c.Assert(err, qt.IsNil)

charlie := env.User("charlie@external").DBObject(c, *s.Database, nil)
charlie := env.User("charlie@external").DBObject(c, *s.Database)

// Add a new user
err = s.Database.UpdateUserCloudAccess(ctx, &dbmodel.UserCloudAccess{
Expand Down Expand Up @@ -553,7 +553,7 @@ controllers:
region: test-region-2
priority: 1
`)
env.PopulateDB(c, *s.Database, nil)
env.PopulateDB(c, *s.Database)

cl := dbmodel.Cloud{
Name: "test-cloud-1",
Expand Down
2 changes: 1 addition & 1 deletion internal/db/cloudcredential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ func (s *dbSuite) TestForEachCloudCredential(c *qt.C) {
env := jimmtest.ParseEnvironment(c, forEachCloudCredentialEnv)
err := s.Database.Migrate(ctx, false)
c.Assert(err, qt.IsNil)
env.PopulateDB(c, *s.Database, nil)
env.PopulateDB(c, *s.Database)

for _, test := range forEachCloudCredentialTests {
c.Run(test.name, func(c *qt.C) {
Expand Down
6 changes: 3 additions & 3 deletions internal/db/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ func (s *dbSuite) TestForEachController(c *qt.C) {
c.Assert(err, qt.Equals, nil)

env := jimmtest.ParseEnvironment(c, testForEachControllerEnv)
env.PopulateDB(c, *s.Database, nil)
env.PopulateDB(c, *s.Database)

testError := errors.E("test error")
err = s.Database.ForEachController(ctx, func(controller *dbmodel.Controller) error {
Expand Down Expand Up @@ -434,9 +434,9 @@ func (s *dbSuite) TestForEachControllerModel(c *qt.C) {
c.Assert(err, qt.Equals, nil)

env := jimmtest.ParseEnvironment(c, testForEachControllerModelEnv)
env.PopulateDB(c, *s.Database, nil)
env.PopulateDB(c, *s.Database)

ctl := env.Controller("test").DBObject(c, *s.Database, nil)
ctl := env.Controller("test").DBObject(c, *s.Database)
testError := errors.E("test error")
err = s.Database.ForEachControllerModel(ctx, &ctl, func(_ *dbmodel.Model) error {
return testError
Expand Down
8 changes: 4 additions & 4 deletions internal/db/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ func (s *dbSuite) TestUpdateUserModelAccess(c *qt.C) {
c.Assert(err, qt.Equals, nil)

env := jimmtest.ParseEnvironment(c, testUpdateUserModelAccessEnv)
env.PopulateDB(c, *s.Database, nil)
env.PopulateDB(c, *s.Database)

m := dbmodel.Model{
UUID: sql.NullString{
Expand All @@ -554,7 +554,7 @@ func (s *dbSuite) TestUpdateUserModelAccess(c *qt.C) {
err = s.Database.GetModel(ctx, &m)
c.Assert(err, qt.IsNil)

charlie := env.User("charlie@external").DBObject(c, *s.Database, nil)
charlie := env.User("charlie@external").DBObject(c, *s.Database)

// Add a new user
err = s.Database.UpdateUserModelAccess(ctx, &dbmodel.UserModelAccess{
Expand Down Expand Up @@ -699,7 +699,7 @@ func (s *dbSuite) TestForEachModel(c *qt.C) {
c.Assert(err, qt.Equals, nil)

env := jimmtest.ParseEnvironment(c, testForEachModelEnv)
env.PopulateDB(c, *s.Database, nil)
env.PopulateDB(c, *s.Database)

testError := errors.E("test error")
err = s.Database.ForEachModel(ctx, func(m *dbmodel.Model) error {
Expand Down Expand Up @@ -785,7 +785,7 @@ func (s *dbSuite) TestGetModelsByUUID(c *qt.C) {
c.Assert(err, qt.Equals, nil)

env := jimmtest.ParseEnvironment(c, testGetModelsByUUIDEnv)
env.PopulateDB(c, *s.Database, nil)
env.PopulateDB(c, *s.Database)

modelUUIDs := []string{
"00000002-0000-0000-0000-000000000001",
Expand Down
4 changes: 2 additions & 2 deletions internal/db/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,9 @@ models:
- user: bob@external
access: "read"
`)
env.PopulateDB(c, *s.Database, nil)
env.PopulateDB(c, *s.Database)

u := env.User("bob@external").DBObject(c, *s.Database, nil)
u := env.User("bob@external").DBObject(c, *s.Database)
models, err := s.Database.GetUserModels(ctx, &u)
c.Assert(err, qt.IsNil)
c.Check(models, jimmtest.DBObjectEquals, []dbmodel.UserModelAccess{{
Expand Down
5 changes: 2 additions & 3 deletions internal/jimm/applicationoffer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2802,7 +2802,6 @@ func TestListApplicationOffers(t *testing.T) {
err = db.Migrate(ctx, false)
c.Assert(err, qt.IsNil)
env := jimmtest.ParseEnvironment(c, listApplicationsTestEnv)
env.PopulateDB(c, db, client)

j := &jimm.JIMM{
UUID: uuid.NewString(),
Expand Down Expand Up @@ -2957,7 +2956,7 @@ func TestListApplicationOffers(t *testing.T) {
},
},
}

env.PopulateDBAndPermissions(c, j.ResourceTag(), db, client)
tuples := []openfga.Tuple{{
Object: ofganames.ConvertTag(names.NewUserTag("alice@external")),
Relation: ofganames.AdministratorRelation,
Expand Down Expand Up @@ -2998,7 +2997,7 @@ func TestListApplicationOffers(t *testing.T) {
err = client.AddRelation(context.Background(), tuples...)
c.Assert(err, qt.IsNil)

u := env.User("alice@external").DBObject(c, db, client)
u := env.User("alice@external").DBObject(c, db)
_, err = j.ListApplicationOffers(ctx, openfga.NewUser(&u, client))
c.Assert(err, qt.ErrorMatches, `at least one filter must be specified`)

Expand Down
30 changes: 14 additions & 16 deletions internal/jimm/cloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -910,10 +910,9 @@ func TestAddHostedCloud(t *testing.T) {
c.Assert(err, qt.IsNil)

env := jimmtest.ParseEnvironment(c, addHostedCloudTestEnv)
env.PopulateDB(c, j.Database, client)
env.AddJIMMRelations(c, j.ResourceTag(), j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)

dbUser := env.User(test.username).DBObject(c, j.Database, client)
dbUser := env.User(test.username).DBObject(c, j.Database)
user := openfga.NewUser(&dbUser, client)

err = j.AddHostedCloud(ctx, user, names.NewCloudTag(test.cloudName), test.cloud, false)
Expand Down Expand Up @@ -1200,10 +1199,9 @@ func TestAddCloudToController(t *testing.T) {
c.Assert(err, qt.IsNil)

env := jimmtest.ParseEnvironment(c, addHostedCloudTestEnv)
env.PopulateDB(c, j.Database, client)
env.AddJIMMRelations(c, j.ResourceTag(), j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)

dbUser := env.User(test.username).DBObject(c, j.Database, client)
dbUser := env.User(test.username).DBObject(c, j.Database)
user := openfga.NewUser(&dbUser, client)

// Note that the force flag has no effect here because the Juju responses are mocked.
Expand Down Expand Up @@ -1360,9 +1358,9 @@ func TestGrantCloudAccess(t *testing.T) {
}
err = j.Database.Migrate(ctx, false)
c.Assert(err, qt.IsNil)
env.PopulateDB(c, j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)

dbUser := env.User(tt.username).DBObject(c, j.Database, client)
dbUser := env.User(tt.username).DBObject(c, j.Database)
user := openfga.NewUser(&dbUser, client)

err = j.GrantCloudAccess(ctx, user, names.NewCloudTag(tt.cloud), names.NewUserTag(tt.targetUsername), tt.access)
Expand Down Expand Up @@ -1660,7 +1658,7 @@ func TestRevokeCloudAccess(t *testing.T) {

err = j.Database.Migrate(ctx, false)
c.Assert(err, qt.IsNil)
env.PopulateDB(c, j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)

if tt.extraInitialTuples != nil && len(tt.extraInitialTuples) > 0 {
err = client.AddRelation(ctx, tt.extraInitialTuples...)
Expand All @@ -1675,7 +1673,7 @@ func TestRevokeCloudAccess(t *testing.T) {
}
}

dbUser := env.User(tt.username).DBObject(c, j.Database, client)
dbUser := env.User(tt.username).DBObject(c, j.Database)
user := openfga.NewUser(&dbUser, client)

err = j.RevokeCloudAccess(ctx, user, names.NewCloudTag(tt.cloud), names.NewUserTag(tt.targetUsername), tt.access)
Expand Down Expand Up @@ -1813,9 +1811,9 @@ func TestRemoveCloud(t *testing.T) {
}
err = j.Database.Migrate(ctx, false)
c.Assert(err, qt.IsNil)
env.PopulateDB(c, j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)

dbUser := env.User(test.username).DBObject(c, j.Database, client)
dbUser := env.User(test.username).DBObject(c, j.Database)
user := openfga.NewUser(&dbUser, client)

err = j.RemoveCloud(ctx, user, names.NewCloudTag(test.cloud))
Expand Down Expand Up @@ -2070,9 +2068,9 @@ func TestUpdateCloud(t *testing.T) {
}
err = j.Database.Migrate(ctx, false)
c.Assert(err, qt.IsNil)
env.PopulateDB(c, j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)

dbUser := env.User(test.username).DBObject(c, j.Database, client)
dbUser := env.User(test.username).DBObject(c, j.Database)
user := openfga.NewUser(&dbUser, client)

tag := names.NewCloudTag(test.cloud)
Expand Down Expand Up @@ -2264,9 +2262,9 @@ func TestRemoveFromControllerCloud(t *testing.T) {
}
err = j.Database.Migrate(ctx, false)
c.Assert(err, qt.IsNil)
env.PopulateDB(c, j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)

dbUser := env.User(test.username).DBObject(c, j.Database, client)
dbUser := env.User(test.username).DBObject(c, j.Database)
user := openfga.NewUser(&dbUser, client)

err = j.RemoveCloudFromController(ctx, user, test.controllerName, names.NewCloudTag(test.cloud))
Expand Down
18 changes: 9 additions & 9 deletions internal/jimm/cloudcredential_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -868,8 +868,8 @@ users:

err = j.Database.Migrate(ctx, false)
c.Assert(err, qt.IsNil)
env.PopulateDB(c, j.Database, client)
u := env.User("alice@external").DBObject(c, j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)
u := env.User("alice@external").DBObject(c, j.Database)

_, err = j.UpdateCloudCredential(ctx, &u, jimm.UpdateCloudCredentialArgs{
CredentialTag: names.NewCloudCredentialTag("test-cloud/bob@external/test"),
Expand Down Expand Up @@ -1540,8 +1540,8 @@ func TestForEachUserCloudCredential(t *testing.T) {
}
err = j.Database.Migrate(ctx, false)
c.Assert(err, qt.IsNil)
env.PopulateDB(c, j.Database, client)
u := env.User(test.username).DBObject(c, j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)
u := env.User(test.username).DBObject(c, j.Database)

var credentials []string
if test.f == nil {
Expand Down Expand Up @@ -1660,12 +1660,12 @@ func TestGetCloudCredentialAttributes(t *testing.T) {
}
err = j.Database.Migrate(ctx, false)
c.Assert(err, qt.IsNil)
env.PopulateDB(c, j.Database, client)
u := env.User("bob@external").DBObject(c, j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)
u := env.User("bob@external").DBObject(c, j.Database)
cred, err := j.GetCloudCredential(ctx, &u, names.NewCloudCredentialTag("test-cloud/bob@external/cred-1"))
c.Assert(err, qt.IsNil)

u = env.User(test.username).DBObject(c, j.Database, client)
u = env.User(test.username).DBObject(c, j.Database)
attr, redacted, err := j.GetCloudCredentialAttributes(ctx, &u, cred, test.hidden)
if test.expectError != "" {
c.Check(err, qt.ErrorMatches, test.expectError)
Expand Down Expand Up @@ -1710,9 +1710,9 @@ func TestCloudCredentialAttributeStore(t *testing.T) {
regions:
- name: test-region
`)
env.PopulateDB(c, j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)

u := env.User("alice@external").DBObject(c, j.Database, client)
u := env.User("alice@external").DBObject(c, j.Database)
args := jimm.UpdateCloudCredentialArgs{
CredentialTag: names.NewCloudCredentialTag("test/alice@external/cred-1"),
Credential: jujuparams.CloudCredential{
Expand Down
31 changes: 13 additions & 18 deletions internal/jimm/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ func TestEarliestControllerVersion(t *testing.T) {
c.Assert(err, qt.IsNil)

env := jimmtest.ParseEnvironment(c, testEarliestControllerVersionEnv)
env.PopulateDB(c, j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)

v, err := j.EarliestControllerVersion(ctx)
c.Assert(err, qt.Equals, nil)
Expand Down Expand Up @@ -915,10 +915,9 @@ func TestImportModel(t *testing.T) {
c.Assert(err, qt.IsNil)

env := jimmtest.ParseEnvironment(c, testImportModelEnv)
env.PopulateDB(c, j.Database, client)
env.AddJIMMRelations(c, j.ResourceTag(), j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)

dbUser := env.User(test.user).DBObject(c, j.Database, client)
dbUser := env.User(test.user).DBObject(c, j.Database)
user := openfga.NewUser(&dbUser, client)
err = j.ImportModel(ctx, user, test.controllerName, names.NewModelTag(test.modelUUID), test.newOwner)
if test.expectedError == "" {
Expand Down Expand Up @@ -1016,10 +1015,9 @@ func TestSetControllerConfig(t *testing.T) {
c.Assert(err, qt.IsNil)

env := jimmtest.ParseEnvironment(c, testControllerConfigEnv)
env.PopulateDB(c, j.Database, client)
env.AddJIMMRelations(c, j.ResourceTag(), j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)

dbUser := env.User(test.user).DBObject(c, j.Database, client)
dbUser := env.User(test.user).DBObject(c, j.Database)
user := openfga.NewUser(&dbUser, client)
err = j.SetControllerConfig(ctx, user, test.args)
if test.expectedError == "" {
Expand Down Expand Up @@ -1092,13 +1090,12 @@ func TestGetControllerConfig(t *testing.T) {
c.Assert(err, qt.IsNil)

env := jimmtest.ParseEnvironment(c, testImportModelEnv)
env.PopulateDB(c, j.Database, client)
env.AddJIMMRelations(c, j.ResourceTag(), j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)

dbSuperuser := env.User("alice@external").DBObject(c, j.Database, client)
dbSuperuser := env.User("alice@external").DBObject(c, j.Database)
superuser := openfga.NewUser(&dbSuperuser, client)

dbUser := env.User(test.user).DBObject(c, j.Database, client)
dbUser := env.User(test.user).DBObject(c, j.Database)
user := openfga.NewUser(&dbUser, client)

err = j.SetControllerConfig(ctx, superuser, jujuparams.ControllerConfigSet{
Expand Down Expand Up @@ -1243,10 +1240,9 @@ func TestUpdateMigratedModel(t *testing.T) {
c.Assert(err, qt.IsNil)

env := jimmtest.ParseEnvironment(c, testUpdateMigratedModelEnv)
env.PopulateDB(c, j.Database, client)
env.AddJIMMRelations(c, j.ResourceTag(), j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)

dbUser := env.User(test.user).DBObject(c, j.Database, client)
dbUser := env.User(test.user).DBObject(c, j.Database)
user := openfga.NewUser(&dbUser, client)

err = j.UpdateMigratedModel(ctx, user, test.model, test.targetController)
Expand Down Expand Up @@ -1297,10 +1293,9 @@ func TestGetControllerAccess(t *testing.T) {
c.Assert(err, qt.IsNil)

env := jimmtest.ParseEnvironment(c, testGetControllerAccessEnv)
env.PopulateDB(c, j.Database, client)
env.AddJIMMRelations(c, j.ResourceTag(), j.Database, client)
env.PopulateDBAndPermissions(c, j.ResourceTag(), j.Database, client)

dbUser := env.User("alice@external").DBObject(c, j.Database, client)
dbUser := env.User("alice@external").DBObject(c, j.Database)
alice := openfga.NewUser(&dbUser, client)

access, err := j.GetJimmControllerAccess(ctx, alice, names.NewUserTag("alice@external"))
Expand All @@ -1315,7 +1310,7 @@ func TestGetControllerAccess(t *testing.T) {
c.Assert(err, qt.IsNil)
c.Check(access, qt.Equals, "login")

dbUser = env.User("bob@external").DBObject(c, j.Database, client)
dbUser = env.User("bob@external").DBObject(c, j.Database)
alice = openfga.NewUser(&dbUser, client)
access, err = j.GetJimmControllerAccess(ctx, alice, names.NewUserTag("bob@external"))
c.Assert(err, qt.IsNil)
Expand Down
Loading

0 comments on commit 8d0dc1f

Please sign in to comment.