Merge pull request #676 from cloudfoundry/part5_final

Golang unit tests can now be run in parallel
cloudfoundry · May 25, 2022 · 2d1a07b · 2d1a07b
2 parents 08f82dc + 07fded1
commit 2d1a07b
Show file tree

Hide file tree

Showing 14 changed files with 135 additions and 166 deletions.
diff --git a/src/autoscaler/Makefile b/src/autoscaler/Makefile
@@ -56,10 +56,10 @@ ginkgo_check:
 
 test: ginkgo_check
 	@echo "Running tests"
-	@APP_AUTOSCALER_TEST_RUN=true ginkgo -v ${GINKGO_OPTS} --skip-package=integration
+	@APP_AUTOSCALER_TEST_RUN=true ginkgo -p ${GINKGO_OPTS} --skip-package=integration
 
 testsuite: ginkgo_check
-	APP_AUTOSCALER_TEST_RUN=true ginkgo -v ${GINKGO_OPTS} ${TEST}
+	APP_AUTOSCALER_TEST_RUN=true ginkgo -p ${GINKGO_OPTS} ${TEST}
 
 .PHONY: integration
 integration: ginkgo_check

diff --git a/src/autoscaler/api/cmd/api/api_suite_test.go b/src/autoscaler/api/cmd/api/api_suite_test.go
@@ -10,6 +10,8 @@ import (
 	"path/filepath"
 	"time"
 
+	. "code.cloudfoundry.org/app-autoscaler/src/autoscaler/testhelpers"
+
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/api/config"
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/cf"
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/db"
@@ -308,8 +310,6 @@ func (ap *ApiRunner) Interrupt() {
 
 func readFile(filename string) string {
 	contents, err := ioutil.ReadFile(filename)
-	if err != nil {
-		Fail("Failed to read file:" + filename + " " + err.Error())
-	}
+	FailOnError("Failed to read file:"+filename+" ", err)
 	return string(contents)
 }
diff --git a/src/autoscaler/api/cmd/api/api_test.go b/src/autoscaler/api/cmd/api/api_test.go
@@ -8,6 +8,8 @@ import (
 	"path/filepath"
 	"time"
 
+	. "code.cloudfoundry.org/app-autoscaler/src/autoscaler/testhelpers"
+
 	"code.cloudfoundry.org/cfhttp"
 
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/api/config"
@@ -33,9 +35,7 @@ var _ = Describe("Api", func() {
 			filepath.Join(testCertDir, "servicebroker.crt"),
 			filepath.Join(testCertDir, "servicebroker.key"),
 			filepath.Join(testCertDir, "autoscaler-ca.crt"))
-		if err != nil {
-			Fail("cfhttp.NewTLSConfig failed:" + err.Error())
-		}
+		FailOnError("cfhttp.NewTLSConfig failed:", err)
 		brokerHttpClient = &http.Client{
 			Transport: &http.Transport{
 				TLSClientConfig: brokerClientTLSConfig,
@@ -180,9 +180,7 @@ var _ = Describe("Api", func() {
 
 				bodyBytes, err := ioutil.ReadAll(rsp.Body)
 
-				if err != nil {
-					Fail("Read failed:" + err.Error())
-				}
+				FailOnError("Read failed:", err)
 				if len(bodyBytes) == 0 {
 					Fail("body empty")
 				}

diff --git a/src/autoscaler/db/sqldb/appmetric_sqldb_test.go b/src/autoscaler/db/sqldb/appmetric_sqldb_test.go
@@ -6,6 +6,8 @@ import (
 	"sync"
 	"time"
 
+	. "code.cloudfoundry.org/app-autoscaler/src/autoscaler/testhelpers"
+
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/db"
 	. "code.cloudfoundry.org/app-autoscaler/src/autoscaler/db/sqldb"
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/models"

diff --git a/src/autoscaler/db/sqldb/lock_sqldb_test.go b/src/autoscaler/db/sqldb/lock_sqldb_test.go
@@ -1,6 +1,7 @@
 package sqldb_test
 
 import (
+	"fmt"
 	"os"
 	"strings"
 	"time"
@@ -25,9 +26,13 @@ var _ = Describe("LockSqldb", func() {
 		lock           *models.Lock
 		isLockAcquired bool
 		testTTL        time.Duration
+		ownerId        string
+		ownerId2       string
 	)
 
 	BeforeEach(func() {
+		ownerId = fmt.Sprintf("111111%d", GinkgoParallelProcess())
+		ownerId2 = fmt.Sprintf("222222%d", GinkgoParallelProcess())
 		logger = lager.NewLogger("lock-sqldb-test")
 		dbConfig = db.DatabaseConfig{
 			URL:                   os.Getenv("DBURL"),
@@ -36,21 +41,29 @@ var _ = Describe("LockSqldb", func() {
 			ConnectionMaxLifetime: 10 * time.Second,
 			ConnectionMaxIdleTime: 10 * time.Second,
 		}
-		testTTL = time.Duration(15) * time.Second
-	})
-
-	Describe("NewLockSQLDB", func() {
-		JustBeforeEach(func() {
-			ldb, err = NewLockSQLDB(dbConfig, "test_lock", logger)
-		})
+		testTTL = 1 * time.Second
 
-		AfterEach(func() {
+		ldb, err = NewLockSQLDB(dbConfig, lockTable, logger)
+		DeferCleanup(func() {
 			if ldb != nil {
 				err = ldb.Close()
 				Expect(err).NotTo(HaveOccurred())
 			}
 		})
 
+	})
+
+	AfterEach(func() {
+		err = cleanLockTable()
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	Describe("NewLockSQLDB", func() {
+		JustBeforeEach(func() {
+			_ = ldb.Close()
+			ldb, err = NewLockSQLDB(dbConfig, lockTable, logger)
+		})
+
 		Context("when db url is not correct", func() {
 			BeforeEach(func() {
 				if !strings.Contains(os.Getenv("DBURL"), "postgres") {
@@ -84,44 +97,32 @@ var _ = Describe("LockSqldb", func() {
 	})
 
 	Describe("Lock", func() {
-		BeforeEach(func() {
-			ldb, err = NewLockSQLDB(dbConfig, "test_lock", logger)
-			Expect(err).NotTo(HaveOccurred())
-		})
-
-		AfterEach(func() {
-			err = ldb.Close()
-			Expect(err).NotTo(HaveOccurred())
-			err = cleanLockTable()
-			Expect(err).NotTo(HaveOccurred())
-		})
-
 		Context("when the lock does not exist", func() {
 			Context("because the row does not exist", func() {
 				BeforeEach(func() {
-					lock = createLock("123456", testTTL)
+					lock = createLock(ownerId, testTTL)
 				})
 
 				It("insert the lock for the owner", func() {
 					isLockAcquired, err = ldb.Lock(lock)
 					Expect(err).NotTo(HaveOccurred())
 					Expect(isLockAcquired).To(BeTrue())
-					Expect(validateLockInDB("123456", lock)).To(Succeed())
+					Expect(validateLockInDB(ownerId, lock)).To(Succeed())
 				})
 			})
 		})
 
 		Context("when the lock exist", func() {
 			Context("and the owner is same", func() {
 				BeforeEach(func() {
-					lock = createLock("213123313", testTTL)
+					lock = createLock(ownerId, testTTL)
 					result, err := insertLockDetails(lock)
 					Expect(err).NotTo(HaveOccurred())
 					Expect(result.RowsAffected()).To(BeEquivalentTo(1))
-					Expect(validateLockInDB("213123313", lock)).To(Succeed())
+					Expect(validateLockInDB(ownerId, lock)).To(Succeed())
 				})
 				It("should successfully renew the lock", func() {
-					lock = createLock("213123313", testTTL)
+					lock = createLock(ownerId, testTTL)
 					isLockAcquired, err = ldb.Lock(lock)
 					Expect(err).NotTo(HaveOccurred())
 					Expect(isLockAcquired).To(BeTrue())
@@ -131,35 +132,35 @@ var _ = Describe("LockSqldb", func() {
 			Context("and the owner is different", func() {
 				Context("and lock recently renewed by owner", func() {
 					BeforeEach(func() {
-						lock = createLock("65432199", testTTL)
+						lock = createLock(ownerId, testTTL)
 						isLockAcquired, err = ldb.Lock(lock)
 						Expect(err).NotTo(HaveOccurred())
 						Expect(isLockAcquired).To(BeTrue())
-						Expect(validateLockInDB("65432199", lock)).To(Succeed())
+						Expect(validateLockInDB(ownerId, lock)).To(Succeed())
 					})
 					It("competing instance should fail to get the lock", func() {
-						lock = createLock("1234567", testTTL)
+						lock = createLock(ownerId2, testTTL)
 						isLockAcquired, err = ldb.Lock(lock)
 						Expect(isLockAcquired).To(BeFalse())
-						Expect(validateLockInDB("1234567", lock)).NotTo(Succeed())
+						Expect(validateLockInDB(ownerId2, lock)).NotTo(Succeed())
 					})
 				})
 
 				Context("and lock expired", func() {
 					BeforeEach(func() {
-						lock = createLock("24165435", testTTL)
+						lock = createLock(ownerId, testTTL)
 						isLockAcquired, err = ldb.Lock(lock)
 						Expect(err).NotTo(HaveOccurred())
 						Expect(isLockAcquired).To(BeTrue())
-						Expect(validateLockInDB("24165435", lock)).To(Succeed())
+						Expect(validateLockInDB(ownerId, lock)).To(Succeed())
 					})
 					It("competing instance should successfully acquire the lock", func() {
-						time.Sleep(testTTL + 5*time.Second) //waiting for the ttl to expire
-						lock = createLock("123456", testTTL)
+						time.Sleep(testTTL + 50*time.Millisecond) //waiting for the ttl to expire
+						lock = createLock(ownerId, testTTL)
 						isLockAcquired, err = ldb.Lock(lock)
 						Expect(err).NotTo(HaveOccurred())
 						Expect(isLockAcquired).To(BeTrue())
-						Expect(validateLockInDB("123456", lock)).To(Succeed())
+						Expect(validateLockInDB(ownerId, lock)).To(Succeed())
 					})
 
 				})
@@ -179,7 +180,7 @@ var _ = Describe("LockSqldb", func() {
 			})
 
 			It("should fail to acquire lock", func() {
-				lock = createLock("123456", testTTL)
+				lock = createLock(ownerId, testTTL)
 				isLockAcquired, err = ldb.Lock(lock)
 				Expect(err).To(HaveOccurred())
 				Expect(isLockAcquired).To(BeFalse())
@@ -189,29 +190,20 @@ var _ = Describe("LockSqldb", func() {
 
 	Describe("Release Lock", func() {
 		BeforeEach(func() {
-			ldb, err = NewLockSQLDB(dbConfig, "test_lock", logger)
-			Expect(err).NotTo(HaveOccurred())
-			lock = createLock("654321", testTTL)
-		})
-
-		AfterEach(func() {
-			err = ldb.Close()
-			Expect(err).NotTo(HaveOccurred())
-			err = cleanLockTable()
-			Expect(err).NotTo(HaveOccurred())
+			lock = createLock(ownerId, testTTL)
 		})
 
 		Context("when the lock exist", func() {
 			BeforeEach(func() {
 				result, err := insertLockDetails(lock)
 				Expect(err).NotTo(HaveOccurred())
 				Expect(result.RowsAffected()).To(BeEquivalentTo(1))
-				Expect(validateLockInDB("654321", lock)).To(Succeed())
+				Expect(validateLockInDB(ownerId, lock)).To(Succeed())
 			})
 			It("removes the lock from the locks table", func() {
 				err = ldb.Release(lock.Owner)
 				Expect(err).NotTo(HaveOccurred())
-				Expect(validateLockNotInDB("654321")).To(Succeed())
+				Expect(validateLockNotInDB(ownerId)).To(Succeed())
 			})
 		})
 

diff --git a/src/autoscaler/db/sqldb/policy_sqldb_test.go b/src/autoscaler/db/sqldb/policy_sqldb_test.go
@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"strings"
 
+	. "code.cloudfoundry.org/app-autoscaler/src/autoscaler/testhelpers"
+
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/db"
 	. "code.cloudfoundry.org/app-autoscaler/src/autoscaler/db/sqldb"
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/models"
@@ -57,9 +59,7 @@ var _ = Describe("PolicySQLDB", func() {
 			ConnectionMaxIdleTime: 10 * time.Second,
 		}
 		pdb, err = NewPolicySQLDB(dbConfig, logger)
-		if err != nil {
-			Fail("unable to connect policy-db: " + err.Error())
-		}
+		FailOnError("unable to connect policy-db: ", err)
 		DeferCleanup(func() error {
 			if pdb != nil {
 				return pdb.Close()

diff --git a/src/autoscaler/db/sqldb/scalingengine_sqldb_test.go b/src/autoscaler/db/sqldb/scalingengine_sqldb_test.go
@@ -1,9 +1,10 @@
 package sqldb_test
 
 import (
-	"fmt"
 	"strings"
 
+	. "code.cloudfoundry.org/app-autoscaler/src/autoscaler/testhelpers"
+
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/db"
 	. "code.cloudfoundry.org/app-autoscaler/src/autoscaler/db/sqldb"
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/models"
@@ -54,9 +55,7 @@ var _ = Describe("ScalingEngineSqldb", func() {
 			ConnectionMaxIdleTime: 10 * time.Second,
 		}
 		sdb, err = NewScalingEngineSQLDB(dbConfig, logger)
-		if err != nil {
-			Fail("Could not open db connection: " + err.Error())
-		}
+		FailOnError("Could not open db connection: ", err)
 		DeferCleanup(func() error {
 			if sdb != nil {
 				return sdb.Close()
@@ -773,12 +772,6 @@ var _ = Describe("ScalingEngineSqldb", func() {
 	})
 })
 
-func FailOnError(message string, err error) {
-	if err != nil {
-		Fail(fmt.Sprintf("%s: %s", message, err.Error()))
-	}
-}
-
 func cleanupForApp(appId string) {
 	removeScalingHistoryForApp(appId)
 	removeCooldownForApp(appId)