From 0f235a8594248d4e75c412e115d62f4514c9d3e8 Mon Sep 17 00:00:00 2001 From: George Blue Date: Mon, 8 Apr 2024 15:47:40 +0100 Subject: [PATCH] test: move "dbservices" tests to Ginkgo style Requested in PR comment for https://github.com/cloudfoundry/cloud-service-broker/pull/983 [#187331719](https://www.pivotaltracker.com/story/show/187331719) --- dbservice/dbservice_suite_test.go | 13 ++ dbservice/migrations_test.go | 217 +++++------------- .../recover_in_progress_operations_test.go | 125 ++++------ dbservice/vcap_test.go | 147 +++++------- 4 files changed, 181 insertions(+), 321 deletions(-) create mode 100644 dbservice/dbservice_suite_test.go diff --git a/dbservice/dbservice_suite_test.go b/dbservice/dbservice_suite_test.go new file mode 100644 index 000000000..1003b3d84 --- /dev/null +++ b/dbservice/dbservice_suite_test.go @@ -0,0 +1,13 @@ +package dbservice_test + +import ( + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestDBService(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "DBService Suite") +} diff --git a/dbservice/migrations_test.go b/dbservice/migrations_test.go index ef648cc7d..eb6f3e616 100644 --- a/dbservice/migrations_test.go +++ b/dbservice/migrations_test.go @@ -1,175 +1,84 @@ -// Copyright 2019 the Service Broker Project Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - package dbservice import ( - "errors" "math" "os" - "reflect" - "testing" + "github.com/cloudfoundry/cloud-service-broker/dbservice/models" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" "gorm.io/driver/sqlite" "gorm.io/gorm" - - "github.com/cloudfoundry/cloud-service-broker/dbservice/models" ) -func TestValidateLastMigration(t *testing.T) { - cases := map[string]struct { - LastMigration int - Expected error - }{ - "new-db": { - LastMigration: -1, - Expected: nil, - }, - "before-v2": { - LastMigration: 0, - Expected: errors.New("migration from broker versions <= 2.0 is no longer supported, upgrade using a v3.x broker then try again"), - }, - "v3-to-v4": { - LastMigration: 1, - Expected: nil, - }, - "v4-to-v4.1": { - LastMigration: 2, - Expected: nil, - }, - "v4.1-to-v4.2": { - LastMigration: 3, - Expected: nil, - }, - "up-to-date": { - LastMigration: numMigrations - 1, - Expected: nil, - }, - "future": { - LastMigration: numMigrations, - Expected: errors.New("the database you're connected to is newer than this tool supports"), - }, - "far-future": { - LastMigration: math.MaxInt32, - Expected: errors.New("the database you're connected to is newer than this tool supports"), - }, - } - - for tn, tc := range cases { - t.Run(tn, func(t *testing.T) { - actual := ValidateLastMigration(tc.LastMigration) - - if !reflect.DeepEqual(actual, tc.Expected) { - t.Errorf("Expected error %v, got %v", tc.Expected, actual) +var _ = Describe("Migrations", func() { + DescribeTable( + "validate last migration", + func(lastMigration int, expectedError string) { + err := ValidateLastMigration(lastMigration) + switch expectedError { + case "": + Expect(err).NotTo(HaveOccurred()) + default: + Expect(err).To(MatchError(expectedError)) } - }) - } -} - -func TestRunMigrations_Failures(t *testing.T) { - cases := map[string]struct { - LastMigration int - Expected error - }{ - "before-v2": { - LastMigration: 0, - Expected: errors.New("migration from broker versions <= 2.0 is no longer supported, upgrade using a v3.x broker then try again"), }, - "future": { - LastMigration: numMigrations, - Expected: errors.New("the database you're connected to is newer than this tool supports"), + Entry("new-db", -1, nil), + Entry("before-v2", 0, "migration from broker versions <= 2.0 is no longer supported, upgrade using a v3.x broker then try again"), + Entry("v3-to-v4", 1, nil), + Entry("v4-to-v4.1", 2, nil), + Entry("v4.1-to-v4.2", 3, nil), + Entry("up-to-date", numMigrations-1, nil), + Entry("future", numMigrations, "the database you're connected to is newer than this tool supports"), + Entry("far-future", math.MaxInt32, "the database you're connected to is newer than this tool supports"), + ) + + DescribeTable( + "RunMigrations() failures", + func(lastMigration int, expectedError string) { + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{}) + Expect(err).NotTo(HaveOccurred()) + + Expect(autoMigrateTables(db, &models.MigrationV1{})).To(Succeed()) + Expect(db.Save(&models.Migration{MigrationID: lastMigration}).Error).To(Succeed()) + Expect(RunMigrations(db)).To(MatchError(expectedError)) }, - "far-future": { - LastMigration: math.MaxInt32, - Expected: errors.New("the database you're connected to is newer than this tool supports"), - }, - } - - for tn, tc := range cases { - t.Run(tn, func(t *testing.T) { - db, err := gorm.Open(sqlite.Open("test.sqlite3"), &gorm.Config{}) - defer func() { - _ = os.Remove("test.sqlite3") - }() - if err != nil { - t.Fatal(err) - } - - if err := autoMigrateTables(db, &models.MigrationV1{}); err != nil { - t.Fatal(err) - } - - if err := db.Save(&models.Migration{MigrationID: tc.LastMigration}).Error; err != nil { - t.Fatal(err) - } - - actual := RunMigrations(db) - if !reflect.DeepEqual(actual, tc.Expected) { - t.Errorf("Expected error %v, got %v", tc.Expected, actual) - } + Entry("before-v2", 0, "migration from broker versions <= 2.0 is no longer supported, upgrade using a v3.x broker then try again"), + Entry("future", numMigrations, "the database you're connected to is newer than this tool supports"), + Entry("far-future", math.MaxInt32, "the database you're connected to is newer than this tool supports"), + ) + + Describe("RunMigrations() behavior", func() { + var db *gorm.DB + + BeforeEach(func() { + var err error + // The tests don't pass when using an ":memory:" database as opposed to a real file. Presumably a GORM feature. + db, err = gorm.Open(sqlite.Open("test.sqlite3"), &gorm.Config{}) + Expect(err).NotTo(HaveOccurred()) + + DeferCleanup(func() { + os.Remove("test.sqlite3") + }) }) - } -} - -func TestRunMigrations(t *testing.T) { - cases := map[string]func(t *testing.T, db *gorm.DB){ - "creates-migrations-table": func(t *testing.T, db *gorm.DB) { - if err := RunMigrations(db); err != nil { - t.Fatal(err) - } - if !db.Migrator().HasTable("migrations") { - t.Error("Expected db to have migrations table") - } - }, - - "applies-all-migrations-when-run": func(t *testing.T, db *gorm.DB) { - if err := RunMigrations(db); err != nil { - t.Fatal(err) - } + It("creates a migration table", func() { + Expect(RunMigrations(db)).To(Succeed()) + Expect(db.Migrator().HasTable("migrations")).To(BeTrue()) + }) + It("applies all migrations when run", func() { + Expect(RunMigrations(db)).To(Succeed()) var storedMigrations []models.Migration - if err := db.Order("id desc").Find(&storedMigrations).Error; err != nil { - t.Fatal(err) - } - + Expect(db.Order("id desc").Find(&storedMigrations).Error).To(Succeed()) lastMigrationNumber := storedMigrations[0].MigrationID - if lastMigrationNumber != numMigrations-1 { - t.Errorf("expected lastMigrationNumber to be %d, got %d", numMigrations-1, lastMigrationNumber) - } - }, - - "can-run-migrations-multiple-times": func(t *testing.T, db *gorm.DB) { - for i := 0; i < 10; i++ { - if err := RunMigrations(db); err != nil { - t.Fatal(err) - } - } - }, - } + Expect(lastMigrationNumber).To(Equal(numMigrations - 1)) + }) - for tn, tc := range cases { - t.Run(tn, func(t *testing.T) { - db, err := gorm.Open(sqlite.Open("test.sqlite3"), &gorm.Config{}) - defer func() { - _ = os.Remove("test.sqlite3") - }() - if err != nil { - t.Fatal(err) + It("can run migrations multiple times", func() { + for range 10 { + Expect(RunMigrations(db)).To(Succeed()) } - - tc(t, db) }) - } -} + }) +}) diff --git a/dbservice/recover_in_progress_operations_test.go b/dbservice/recover_in_progress_operations_test.go index ff14d2b37..6401ce63b 100644 --- a/dbservice/recover_in_progress_operations_test.go +++ b/dbservice/recover_in_progress_operations_test.go @@ -1,83 +1,60 @@ package dbservice import ( - "strings" - "testing" - "code.cloudfoundry.org/lager/v3/lagertest" "github.com/cloudfoundry/cloud-service-broker/dbservice/models" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" "gorm.io/driver/sqlite" "gorm.io/gorm" ) -func TestRecoverInProgressOperations(t *testing.T) { - // Setup - db, err := gorm.Open(sqlite.Open(":memory:"), nil) - if err != nil { - t.Errorf("failed to create test database: %s", err) - } - - if err = db.Migrator().CreateTable(&models.TerraformDeployment{}); err != nil { - t.Errorf("failed to create test table: %s", err) - } - - const recoverID = "fake-id-to-recover" - err = db.Create(&models.TerraformDeployment{ - ID: recoverID, - LastOperationType: "fake-type", - LastOperationState: "in progress", - LastOperationMessage: "fake-type in progress", - }).Error - if err != nil { - t.Errorf("failed to create test database data: %s", err) - } - const okID = "fake-id-that-does-not-need-to-be-recovered" - err = db.Create(&models.TerraformDeployment{ - ID: okID, - LastOperationType: "fake-type", - LastOperationState: "succeeded", - LastOperationMessage: "fake-type succeeded", - }).Error - if err != nil { - t.Errorf("failed to create test database data: %s", err) - } - - // Call the function - logger := lagertest.NewTestLogger("test") - recoverInProgressOperations(db, logger) - - // It marks the in-progress operation as failed - var r1 models.TerraformDeployment - err = db.Where("id = ?", recoverID).First(&r1).Error - if err != nil { - t.Errorf("failed to load updated test data: %s", err) - } - - const expState = "failed" - if r1.LastOperationState != expState { - t.Errorf("LastOperationState, expected %q, got %q", expState, r1.LastOperationState) - } - - const expMessage = "the broker restarted while the operation was in progress" - if r1.LastOperationMessage != expMessage { - t.Errorf("LastOperationMessage, expected %q, got %q", expMessage, r1.LastOperationMessage) - } - - // It does not update other operations - var r2 models.TerraformDeployment - err = db.Where("id = ?", okID).First(&r2).Error - if err != nil { - t.Errorf("failed to load updated test data: %s", err) - } - if r2.LastOperationState != "succeeded" || r2.LastOperationMessage != "fake-type succeeded" { - t.Error("row corruption") - } - - // It logs the expected message - const expLog1 = `"message":"test.recover-in-progress-operations.mark-as-failed"` - const expLog2 = `"workspace_id":"fake-id-to-recover"` - logMessage := string(logger.Buffer().Contents()) - if !strings.Contains(logMessage, expLog1) || !strings.Contains(logMessage, expLog2) { - t.Errorf("log, expected to contain %q and %q, got %q", expLog1, expLog2, logMessage) - } -} +var _ = Describe("RecoverInProgressOperations()", func() { + It("recovers the expected operations", func() { + const ( + recoverID = "fake-id-to-recover" + okID = "fake-id-that-does-not-need-to-be-recovered" + ) + + // Setup + db, err := gorm.Open(sqlite.Open(":memory:"), nil) + Expect(err).NotTo(HaveOccurred()) + Expect(db.Migrator().CreateTable(&models.TerraformDeployment{})).To(Succeed()) + + Expect(db.Create(&models.TerraformDeployment{ + ID: recoverID, + LastOperationType: "fake-type", + LastOperationState: "in progress", + LastOperationMessage: "fake-type in progress", + }).Error).To(Succeed()) + Expect(db.Create(&models.TerraformDeployment{ + ID: okID, + LastOperationType: "fake-type", + LastOperationState: "succeeded", + LastOperationMessage: "fake-type succeeded", + }).Error).To(Succeed()) + + // Call the function + logger := lagertest.NewTestLogger("test") + recoverInProgressOperations(db, logger) + + // Behaviors + By("marking the in-progress operation as failed") + var r1 models.TerraformDeployment + Expect(db.Where("id = ?", recoverID).First(&r1).Error).To(Succeed()) + Expect(r1.LastOperationState).To(Equal("failed")) + Expect(r1.LastOperationMessage).To(Equal("the broker restarted while the operation was in progress")) + + By("no updating other operations") + var r2 models.TerraformDeployment + Expect(db.Where("id = ?", okID).First(&r2).Error).To(Succeed()) + Expect(r2.LastOperationState).To(Equal("succeeded")) + Expect(r2.LastOperationMessage).To(Equal("fake-type succeeded")) + + By("logging the expected message") + Expect(logger.Buffer().Contents()).To(SatisfyAll( + ContainSubstring(`"message":"test.recover-in-progress-operations.mark-as-failed"`), + ContainSubstring(`"workspace_id":"fake-id-to-recover"`), + )) + }) +}) diff --git a/dbservice/vcap_test.go b/dbservice/vcap_test.go index 2cb9f669c..a647e8227 100644 --- a/dbservice/vcap_test.go +++ b/dbservice/vcap_test.go @@ -15,11 +15,11 @@ package dbservice import ( - "errors" "fmt" "os" - "testing" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" "github.com/spf13/viper" ) @@ -61,17 +61,15 @@ func ExampleUseVcapServices() { // service_instance_db } -func TestParseVcapServices(t *testing.T) { - cases := map[string]struct { - VcapServiceData string - ExpectedError error - }{ - "empty vcap service": { - VcapServiceData: "", - ExpectedError: errors.New("error unmarshalling VCAP_SERVICES: unexpected end of JSON input"), - }, - "google-cloud vcap-service": { - VcapServiceData: `{ +var _ = Describe("VCAP", func() { + Describe("parsing VCAP_SERVICES", func() { + It("fails when VCAP_SERVICES is empty", func() { + _, err := ParseVcapServices("") + Expect(err).To(MatchError("error unmarshalling VCAP_SERVICES: unexpected end of JSON input")) + }) + + It("parses VCAP_SERVICES for Google Cloud", func() { + _, err := ParseVcapServices(`{ "google-cloudsql-mysql": [ { "binding_name": "testbinding", @@ -106,11 +104,12 @@ func TestParseVcapServices(t *testing.T) { } } ] -}`, - ExpectedError: nil, - }, - "pivotal vcap service": { - VcapServiceData: `{ +}`) + Expect(err).NotTo(HaveOccurred()) + }) + + It("parses VCAP_SERVICES for 'p.mysql' tile", func() { + _, err := ParseVcapServices(`{ "p.mysql": [ { "label": "p.mysql", @@ -133,12 +132,12 @@ func TestParseVcapServices(t *testing.T) { "volume_mounts": [] } ] -} -`, - ExpectedError: nil, - }, - "invalid vcap service - more than one mysql tag": { - VcapServiceData: `{ +}`) + Expect(err).NotTo(HaveOccurred()) + }) + + It("fails when VCAP_SERVICES has more than one MySQL tag", func() { + _, err := ParseVcapServices(`{ "google-cloudsql-mysql": [ { "binding_name": "testbinding", @@ -193,11 +192,12 @@ func TestParseVcapServices(t *testing.T) { "volume_mounts": [] } ] -}`, - ExpectedError: errors.New("error finding MySQL tag: the variable VCAP_SERVICES must have one VCAP service with a tag of 'mysql'. There are currently 2 VCAP services with the tag 'mysql'"), - }, - "invalid vcap service - zero mysql tags": { - VcapServiceData: `{ +}`) + Expect(err).To(MatchError("error finding MySQL tag: the variable VCAP_SERVICES must have one VCAP service with a tag of 'mysql'. There are currently 2 VCAP services with the tag 'mysql'")) + }) + + It("fails when VCAP_SERVICES has zero MySQL tag", func() { + _, err := ParseVcapServices(`{ "p.mysql": [ { "label": "p.mysql", @@ -221,34 +221,18 @@ func TestParseVcapServices(t *testing.T) { } ] } -`, - ExpectedError: errors.New("error finding MySQL tag: the variable VCAP_SERVICES must have one VCAP service with a tag of 'mysql'. There are currently 0 VCAP services with the tag 'mysql'"), - }, - } - - for tn, tc := range cases { - t.Run(tn, func(t *testing.T) { - vcapService, err := ParseVcapServices(tc.VcapServiceData) - if err == nil { - fmt.Printf("\n%#v\n", vcapService) - } - expectError(t, tc.ExpectedError, err) +`) + Expect(err).To(MatchError("error finding MySQL tag: the variable VCAP_SERVICES must have one VCAP service with a tag of 'mysql'. There are currently 0 VCAP services with the tag 'mysql'")) }) - } + }) -} + Describe("setting database credentials", func() { + It("succeeds when the parsed VCAP_SERVICES is empty", func() { + Expect(SetDatabaseCredentials(VcapService{})).To(Succeed()) + }) -func TestSetDatabaseCredentials(t *testing.T) { - cases := map[string]struct { - VcapService VcapService - ExpectedError error - }{ - "empty vcap service": { - VcapService: VcapService{}, - ExpectedError: nil, - }, - "valid gcp vcap service": { - VcapService: VcapService{ + It("succeeds when the parsed VCAP_SERVICES represents a valid Google Cloud service", func() { + vs := VcapService{ BindingName: "testbinding", InstanceName: "testinstance", Name: "kf-binding-tt2-mystorage", @@ -275,11 +259,12 @@ func TestSetDatabaseCredentials(t *testing.T) { "region": "", "uri": "mysql://newuseraccount:Ukd7QEmrfC7xMRqNmTzHCbNnmBtNceys1olOzLoSm4k@104.154.90.3/service_broker?ssl_mode=required", }, - }, - ExpectedError: nil, - }, - "valid pivotal vcap service": { - VcapService: VcapService{ + } + Expect(SetDatabaseCredentials(vs)).To(Succeed()) + }) + + It("succeeds when the parsed VCAP_SERVICES represents a valid 'p.mysql' tile service", func() { + vs := VcapService{ BindingName: "", InstanceName: "", Name: "my-instance", @@ -295,11 +280,12 @@ func TestSetDatabaseCredentials(t *testing.T) { "uri": "mysql://fefcbe8360854a18a7994b870e7b0bf5:z9z6eskdbs1rhtxt@10.0.0.20:3306/service_instance_db?reconnect=true", "username": "fefcbe8360854a18a7994b870e7b0bf5", }, - }, - ExpectedError: nil, - }, - "invalid vcap service - malformed uri": { - VcapService: VcapService{ + } + Expect(SetDatabaseCredentials(vs)).To(Succeed()) + }) + + It("fails when there is an malformed URI", func() { + vs := VcapService{ BindingName: "", InstanceName: "", Name: "my-instance", @@ -315,33 +301,8 @@ func TestSetDatabaseCredentials(t *testing.T) { "uri": "mys@!ql://fefcbe8360854a18a7994b870e7b0bf5:z9z6eskdbs1rhtxt@10.0.0.20:3306/service_instance_db?reconnect=true", "username": "fefcbe8360854a18a7994b870e7b0bf5", }, - }, - ExpectedError: errors.New(`error parsing credentials uri field: parse "mys@!ql://fefcbe8360854a18a7994b870e7b0bf5:z9z6eskdbs1rhtxt@10.0.0.20:3306/service_instance_db?reconnect=true": first path segment in URL cannot contain colon`), - }, - } - - for tn, tc := range cases { - t.Run(tn, func(t *testing.T) { - err := SetDatabaseCredentials(tc.VcapService) - expectError(t, tc.ExpectedError, err) + } + Expect(SetDatabaseCredentials(vs)).To(MatchError(`error parsing credentials uri field: parse "mys@!ql://fefcbe8360854a18a7994b870e7b0bf5:z9z6eskdbs1rhtxt@10.0.0.20:3306/service_instance_db?reconnect=true": first path segment in URL cannot contain colon`)) }) - } - -} - -func expectError(t *testing.T, expected, actual error) { - t.Helper() - expectedErr := expected != nil - gotErr := actual != nil - - switch { - case expectedErr && gotErr: - if expected.Error() != actual.Error() { - t.Fatalf("Expected: %v, got: %v", expected, actual) - } - case expectedErr && !gotErr: - t.Fatalf("Expected: %v, got: %v", expected, actual) - case !expectedErr && gotErr: - t.Fatalf("Expected no error but got: %v", actual) - } -} + }) +})