From 130c4878a473ba781474b21058e895613a8407fd Mon Sep 17 00:00:00 2001 From: George Blue Date: Fri, 26 Jan 2024 12:48:35 +0000 Subject: [PATCH] test: more consistent error handling in integration tests In the integration tests we were using the `must()` helper and also the `Expect(err).NotTo(HaveOccured())` pattern. Both are ok, but the code is cleaner when we are more consistent, and the `must()` pattern is more consise, so the tests were updated to use this pattern wherever possible. --- integrationtest/brokerpak_build_test.go | 6 ++--- .../brokerpaktestframework_test.go | 24 +++++++------------ .../global_labels_propagation_test.go | 3 +-- integrationtest/info_endpoint_test.go | 6 ++--- integrationtest/integrationtest_suite_test.go | 12 ++++------ integrationtest/maintenance_info_test.go | 3 +-- 6 files changed, 19 insertions(+), 35 deletions(-) diff --git a/integrationtest/brokerpak_build_test.go b/integrationtest/brokerpak_build_test.go index dc5faa9e2..8e7dbaaa6 100644 --- a/integrationtest/brokerpak_build_test.go +++ b/integrationtest/brokerpak_build_test.go @@ -38,11 +38,9 @@ var _ = Describe("Brokerpak Build", func() { extractionPath := GinkgoT().TempDir() By("unzipping the brokerpak") - zr, err := zippy.Open(brokerpakPath) - Expect(err).NotTo(HaveOccurred()) + zr := must(zippy.Open(brokerpakPath)) - err = zr.ExtractDirectory("", extractionPath) - Expect(err).NotTo(HaveOccurred()) + Expect(zr.ExtractDirectory("", extractionPath)).To(Succeed()) By("checking that the expected files are there") paths := []string{ diff --git a/integrationtest/brokerpaktestframework_test.go b/integrationtest/brokerpaktestframework_test.go index b8b5b1d65..691dc495e 100644 --- a/integrationtest/brokerpaktestframework_test.go +++ b/integrationtest/brokerpaktestframework_test.go @@ -1,4 +1,4 @@ -package integrationtest +package integrationtest_test import ( "os" @@ -12,18 +12,15 @@ import ( var _ = Describe("brokerpaktestframework", func() { It("works", func() { By("creating a mock Terraform") - mockTerraform, err := brokerpaktestframework.NewTerraformMock(brokerpaktestframework.WithVersion("1.2.3")) - Expect(err).NotTo(HaveOccurred()) + mockTerraform := must(brokerpaktestframework.NewTerraformMock(brokerpaktestframework.WithVersion("1.2.3"))) By("building a fake brokerpak") - cwd, err := os.Getwd() - Expect(err).NotTo(HaveOccurred()) - broker, err := brokerpaktestframework.BuildTestInstance( + cwd := must(os.Getwd()) + broker := must(brokerpaktestframework.BuildTestInstance( filepath.Join(cwd, "fixtures", "brokerpaktestframework"), mockTerraform, GinkgoWriter, - ) - Expect(err).NotTo(HaveOccurred()) + )) By("starting the broker") Expect(broker.Start(GinkgoWriter, nil)).To(Succeed()) @@ -32,16 +29,14 @@ var _ = Describe("brokerpaktestframework", func() { }) By("testing catalog") - catalog, err := broker.Catalog() - Expect(err).NotTo(HaveOccurred()) + catalog := must(broker.Catalog()) Expect(catalog.Services).To(HaveLen(1)) Expect(catalog.Services[0].Name).To(Equal("alpha-service")) By("testing provision") - _, err = broker.Provision("does-not-exist", "does-not-exist", nil) + _, err := broker.Provision("does-not-exist", "does-not-exist", nil) Expect(err).To(MatchError(`cannot find service "does-not-exist" and plan "does-not-exist" in catalog`)) - serviceInstanceID, err := broker.Provision("alpha-service", "alpha", nil) - Expect(err).NotTo(HaveOccurred()) + serviceInstanceID := must(broker.Provision("alpha-service", "alpha", nil)) Expect(serviceInstanceID).To(HaveLen(36)) By("testing bind") @@ -49,8 +44,7 @@ var _ = Describe("brokerpaktestframework", func() { Expect(err).To(MatchError(`cannot find service "does-not-exist" and plan "does-not-exist" in catalog`)) _, err = broker.Bind("alpha-service", "alpha", "does-not-exist", nil) Expect(err).To(MatchError(ContainSubstring(`error retrieving service instance details: could not find service instance details for: does-not-exist`))) - creds, err := broker.Bind("alpha-service", "alpha", serviceInstanceID, nil) - Expect(err).NotTo(HaveOccurred()) + creds := must(broker.Bind("alpha-service", "alpha", serviceInstanceID, nil)) Expect(creds).To(BeEmpty()) }) }) diff --git a/integrationtest/global_labels_propagation_test.go b/integrationtest/global_labels_propagation_test.go index 0444ac7b7..8e76fc41c 100644 --- a/integrationtest/global_labels_propagation_test.go +++ b/integrationtest/global_labels_propagation_test.go @@ -48,9 +48,8 @@ var _ = Describe("Global labels propagation", Label("global-labels"), func() { Outputs map[string]any `json:"outputs"` } - err := dbConn.Where("id = ?", fmt.Sprintf("tf:%s:", serviceInstanceGUID)).First(&tfDeploymentReceiver).Error + Expect(dbConn.Where("id = ?", fmt.Sprintf("tf:%s:", serviceInstanceGUID)).First(&tfDeploymentReceiver).Error).To(Succeed()) - Expect(err).NotTo(HaveOccurred()) Expect(json.Unmarshal(tfDeploymentReceiver.Workspace, &workspaceReceiver)).NotTo(HaveOccurred()) Expect(json.Unmarshal(workspaceReceiver.State, &stateReceiver)).NotTo(HaveOccurred()) return stateReceiver.Outputs diff --git a/integrationtest/info_endpoint_test.go b/integrationtest/info_endpoint_test.go index e9c8e1b47..4a8946472 100644 --- a/integrationtest/info_endpoint_test.go +++ b/integrationtest/info_endpoint_test.go @@ -27,13 +27,11 @@ var _ = Describe("Info Endpoint", func() { }) It("responds to the info endpoint", func() { - resp, err := http.Get(fmt.Sprintf("http://localhost:%d/info", broker.Port)) - Expect(err).NotTo(HaveOccurred()) + resp := must(http.Get(fmt.Sprintf("http://localhost:%d/info", broker.Port))) Expect(resp).To(HaveHTTPStatus(http.StatusOK)) defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) - Expect(err).NotTo(HaveOccurred()) + body := must(io.ReadAll(resp.Body)) var data map[string]any Expect(json.Unmarshal(body, &data)).NotTo(HaveOccurred()) diff --git a/integrationtest/integrationtest_suite_test.go b/integrationtest/integrationtest_suite_test.go index e72e1e889..d69cb6867 100644 --- a/integrationtest/integrationtest_suite_test.go +++ b/integrationtest/integrationtest_suite_test.go @@ -31,15 +31,13 @@ var _ = SynchronizedBeforeSuite( func() []byte { // -gcflags enabled "gops", but had to be removed as this doesn't compile with Go 1.19 //path, err := Build("github.com/cloudfoundry/cloud-service-broker", `-gcflags="all=-N -l"`) - path, err := Build("github.com/cloudfoundry/cloud-service-broker") - Expect(err).NotTo(HaveOccurred()) + path := must(Build("github.com/cloudfoundry/cloud-service-broker")) return []byte(path) }, func(data []byte) { csb = string(data) - cwd, err := os.Getwd() - Expect(err).NotTo(HaveOccurred()) + cwd := must(os.Getwd()) fixtures = func(name string) string { return filepath.Join(cwd, "fixtures", name) } @@ -52,8 +50,7 @@ var _ = SynchronizedAfterSuite( ) var _ = BeforeEach(func() { - fh, err := os.CreateTemp(os.TempDir(), "csbdb") - Expect(err).NotTo(HaveOccurred()) + fh := must(os.CreateTemp(os.TempDir(), "csbdb")) defer fh.Close() database = fh.Name() @@ -61,8 +58,7 @@ var _ = BeforeEach(func() { cleanup(database) }) - dbConn, err = gorm.Open(sqlite.Open(database), &gorm.Config{}) - Expect(err).NotTo(HaveOccurred()) + dbConn = must(gorm.Open(sqlite.Open(database), &gorm.Config{})) }) func requestID() string { diff --git a/integrationtest/maintenance_info_test.go b/integrationtest/maintenance_info_test.go index dcaed067d..de0778b77 100644 --- a/integrationtest/maintenance_info_test.go +++ b/integrationtest/maintenance_info_test.go @@ -42,8 +42,7 @@ var _ = Describe("Maintenance Info", func() { Expect(catalogResponse.Error).NotTo(HaveOccurred()) var catServices apiresponses.CatalogResponse - err := json.Unmarshal(catalogResponse.ResponseBody, &catServices) - Expect(err).NotTo(HaveOccurred()) + Expect(json.Unmarshal(catalogResponse.ResponseBody, &catServices)).To(Succeed()) Expect(catServices.Services[0].Plans[0].MaintenanceInfo).To(Equal(&domain.MaintenanceInfo{ Public: nil, Private: "",