From 5ded876d658d6da919d659f653a47e5762628ac2 Mon Sep 17 00:00:00 2001 From: SimoneDutto Date: Wed, 4 Sep 2024 12:55:15 +0200 Subject: [PATCH] improve tests --- internal/db/resources.go | 46 +++++++++---------- internal/db/resources_test.go | 14 +++--- internal/jimm/resource_test.go | 8 ++-- .../rebac_admin/resources_integration_test.go | 41 +++++++++++------ 4 files changed, 61 insertions(+), 48 deletions(-) diff --git a/internal/db/resources.go b/internal/db/resources.go index 68843bbef..7c60298d4 100644 --- a/internal/db/resources.go +++ b/internal/db/resources.go @@ -11,27 +11,6 @@ import ( // MULTI_TABLES_RAW_SQL contains the raw query fetching entities from multiple tables, with their respective entity parents. const MULTI_TABLES_RAW_SQL = ` -( - SELECT 'controller' AS type, - controllers.uuid AS id, - controllers.name AS name, - '' AS parent_id, - '' AS parent_name, - '' AS parent_type - FROM controllers -) -UNION -( - SELECT 'model' AS type, - models.uuid AS id, - models.name AS name, - controllers.uuid AS parent_id, - controllers.name AS parent_name, - 'controller' AS parent_type - FROM models - JOIN controllers ON models.controller_id = controllers.id -) -UNION ( SELECT 'application_offer' AS type, application_offers.uuid AS id, @@ -53,6 +32,27 @@ UNION FROM clouds ) UNION +( + SELECT 'controller' AS type, + controllers.uuid AS id, + controllers.name AS name, + '' AS parent_id, + '' AS parent_name, + '' AS parent_type + FROM controllers +) +UNION +( + SELECT 'model' AS type, + models.uuid AS id, + models.name AS name, + controllers.uuid AS parent_id, + controllers.name AS parent_name, + 'controller' AS parent_type + FROM models + JOIN controllers ON models.controller_id = controllers.id +) +UNION ( SELECT 'service_account' AS type, identities.name AS id, @@ -61,9 +61,9 @@ UNION '' AS parent_name, '' AS parent_type FROM identities - WHERE name NOT LIKE '%@%' + WHERE name LIKE '%@serviceaccount' ) -ORDER BY id +ORDER BY type, id OFFSET ? LIMIT ?; ` diff --git a/internal/db/resources_test.go b/internal/db/resources_test.go index 2fd228658..b66f8530e 100644 --- a/internal/db/resources_test.go +++ b/internal/db/resources_test.go @@ -12,10 +12,7 @@ import ( "github.com/canonical/jimm/v3/internal/dbmodel" ) -func (s *dbSuite) Setup(c *qt.C) (dbmodel.Model, dbmodel.Controller, dbmodel.Cloud) { - err := s.Database.Migrate(context.Background(), true) - c.Assert(err, qt.Equals, nil) - +func (s *dbSuite) SetupDB(c *qt.C) (dbmodel.Model, dbmodel.Controller, dbmodel.Cloud) { u, err := dbmodel.NewIdentity("bob@canonical.com") c.Assert(err, qt.IsNil) c.Assert(s.Database.DB.Create(&u).Error, qt.IsNil) @@ -73,11 +70,16 @@ func (s *dbSuite) Setup(c *qt.C) (dbmodel.Model, dbmodel.Controller, dbmodel.Clo } func (s *dbSuite) TestGetResources(c *qt.C) { - // create one model, one controller, one cloud - model, controller, cloud := s.Setup(c) ctx := context.Background() + err := s.Database.Migrate(context.Background(), true) + c.Assert(err, qt.Equals, nil) res, err := s.Database.ListResources(ctx, 10, 0) c.Assert(err, qt.Equals, nil) + c.Assert(res, qt.HasLen, 0) + // create one model, one controller, one cloud + model, controller, cloud := s.SetupDB(c) + res, err = s.Database.ListResources(ctx, 10, 0) + c.Assert(err, qt.Equals, nil) c.Assert(res, qt.HasLen, 3) for _, r := range res { switch r.Type { diff --git a/internal/jimm/resource_test.go b/internal/jimm/resource_test.go index 5e75b01ec..a6c78d2ea 100644 --- a/internal/jimm/resource_test.go +++ b/internal/jimm/resource_test.go @@ -3,7 +3,6 @@ package jimm_test import ( "context" - "slices" "testing" "time" @@ -35,10 +34,9 @@ func TestGetResources(t *testing.T) { err = j.Database.Migrate(ctx, false) c.Assert(err, qt.IsNil) - user, _, controller, model, applicationOffer, cloud, _ := createTestControllerEnvironment(ctx, c, j.Database) + _, _, controller, model, applicationOffer, cloud, _ := createTestControllerEnvironment(ctx, c, j.Database) - ids := []string{user.Name, controller.UUID, model.UUID.String, applicationOffer.UUID, cloud.Name} - slices.Sort(ids) + ids := []string{applicationOffer.UUID, cloud.Name, controller.UUID, model.UUID.String} u := openfga.NewUser(&dbmodel.Identity{Name: "admin@canonical.com"}, ofgaClient) u.JimmAdmin = true @@ -59,7 +57,7 @@ func TestGetResources(t *testing.T) { desc: "test with remianing ids", limit: 3, offset: 3, - identities: []string{ids[3], ids[4]}, + identities: []string{ids[3]}, }, { desc: "test out of range", diff --git a/internal/rebac_admin/resources_integration_test.go b/internal/rebac_admin/resources_integration_test.go index 7110bbf63..d19b1f77c 100644 --- a/internal/rebac_admin/resources_integration_test.go +++ b/internal/rebac_admin/resources_integration_test.go @@ -3,7 +3,6 @@ package rebac_admin_test import ( "context" - "slices" rebac_handlers "github.com/canonical/rebac-admin-ui-handlers/v1" "github.com/canonical/rebac-admin-ui-handlers/v1/resources" @@ -73,19 +72,30 @@ func (s *resourcesSuite) TestPatchIdentityEntitlements(c *gc.C) { tester := jimmtest.GocheckTester{C: c} env := jimmtest.ParseEnvironment(tester, resourcesTestEnv) env.PopulateDB(tester, s.JIMM.Database) - ids := make([]string, 0) - for _, m := range env.Models { - ids = append(ids, m.UUID) + type testEntity struct { + Id string + ParentId string + } + ids := make([]testEntity, 0) + for _, c := range env.Clouds { + ids = append(ids, testEntity{ + Id: c.Name, + ParentId: "", + }) } for _, c := range env.Controllers { - ids = append(ids, c.UUID) + ids = append(ids, testEntity{ + Id: c.UUID, + ParentId: "", + }) } - for _, c := range env.Clouds { - ids = append(ids, c.Name) + for _, m := range env.Models { + ids = append(ids, testEntity{ + Id: m.UUID, + ParentId: env.Controller(m.Controller).UUID, + }) } - slices.Sort(ids) - testCases := []struct { desc string size *int @@ -93,7 +103,7 @@ func (s *resourcesSuite) TestPatchIdentityEntitlements(c *gc.C) { wantPage int wantSize int wantNextpage *int - ids []string + ids []testEntity }{ { desc: "test with first page", @@ -102,7 +112,7 @@ func (s *resourcesSuite) TestPatchIdentityEntitlements(c *gc.C) { wantPage: 0, wantSize: 2, wantNextpage: utils.IntToPointer(1), - ids: []string{ids[0], ids[1]}, + ids: []testEntity{ids[0], ids[1]}, }, { desc: "test with second page", @@ -111,7 +121,7 @@ func (s *resourcesSuite) TestPatchIdentityEntitlements(c *gc.C) { wantPage: 1, wantSize: 2, wantNextpage: utils.IntToPointer(2), - ids: []string{ids[2], ids[3]}, + ids: []testEntity{ids[2], ids[3]}, }, { desc: "test with last page", @@ -120,7 +130,7 @@ func (s *resourcesSuite) TestPatchIdentityEntitlements(c *gc.C) { wantPage: 2, wantSize: 1, wantNextpage: nil, - ids: []string{ids[4]}, + ids: []testEntity{ids[4]}, }, } for _, t := range testCases { @@ -138,7 +148,10 @@ func (s *resourcesSuite) TestPatchIdentityEntitlements(c *gc.C) { c.Assert(*resources.Next.Page, gc.Equals, *t.wantNextpage) } for i := range len(t.ids) { - c.Assert(resources.Data[i].Entity.Id, gc.Equals, t.ids[i]) + c.Assert(resources.Data[i].Entity.Id, gc.Equals, t.ids[i].Id) + if t.ids[i].ParentId != "" { + c.Assert(resources.Data[i].Parent.Id, gc.Equals, t.ids[i].ParentId) + } } }