Skip to content

Commit

Permalink
test: more consistent error handling in integration tests (#938)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
blgm authored Jan 29, 2024
1 parent 90df7a5 commit 08c9983
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 35 deletions.
6 changes: 2 additions & 4 deletions integrationtest/brokerpak_build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
24 changes: 9 additions & 15 deletions integrationtest/brokerpaktestframework_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package integrationtest
package integrationtest_test

import (
"os"
Expand All @@ -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())
Expand All @@ -32,25 +29,22 @@ 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")
_, err = broker.Bind("does-not-exist", "does-not-exist", "does-not-exist", nil)
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())
})
})
3 changes: 1 addition & 2 deletions integrationtest/global_labels_propagation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions integrationtest/info_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())

Expand Down
12 changes: 4 additions & 8 deletions integrationtest/integrationtest_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -52,17 +50,15 @@ 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()
DeferCleanup(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 {
Expand Down
3 changes: 1 addition & 2 deletions integrationtest/maintenance_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: "",
Expand Down

0 comments on commit 08c9983

Please sign in to comment.