Skip to content

Commit

Permalink
Fix flaky test
Browse files Browse the repository at this point in the history
The flaky test tries to check that the package cleaner has been called
for the app that the test package belongs to. However the package
cleaner is shared accorss the whole workloads suite, meaning that the
package controller can be triggered for other apps' packages. Perhaps
this is what was causing the flake:

- The test waits for a new call of the cleaner (eventually call count is
  bumped)
- Then the test checks that the cleaner was called for the app owning
  the test package, but the app is not the expected one because some
  other test dealing with packages is running in parallel

Putting both checks under the same eventually clause will make the test
retry until both conditions are true. We have to also iterate over all
calls starting from the first recorded one up to
packageCleaner.CleanCallCount() in case several packages were cleanded
since our last check.

Here are some historical instances of this flake:
- https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-tests-periodic/builds/13588
- https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-tests-periodic/builds/11624
- https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-tests-periodic/builds/11488
- https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-tests-periodic/builds/10299
  • Loading branch information
georgethebeatle authored and danail-branekov committed Nov 28, 2023
1 parent 88c25a2 commit 9d0841e
Showing 1 changed file with 6 additions and 4 deletions.
10 changes: 6 additions & 4 deletions controllers/controllers/workloads/cfpackage_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,13 @@ var _ = Describe("CFPackageReconciler Integration Tests", func() {
It("deletes the older packages for the same app", func() {
Eventually(func(g Gomega) {
g.Expect(packageCleaner.CleanCallCount()).To(BeNumerically(">", cleanCallCount))
}).Should(Succeed())

_, app := packageCleaner.CleanArgsForCall(cleanCallCount)
Expect(app.Name).To(Equal(cfAppGUID))
Expect(app.Namespace).To(Equal(cfSpace.Status.GUID))
for currCall := cleanCallCount; currCall < packageCleaner.CleanCallCount(); currCall++ {
_, app := packageCleaner.CleanArgsForCall(currCall)
g.Expect(app.Name).To(Equal(cfAppGUID))
g.Expect(app.Namespace).To(Equal(cfSpace.Status.GUID))
}
}).Should(Succeed())
})

It("sets the ObservedGeneration status field", func() {
Expand Down

0 comments on commit 9d0841e

Please sign in to comment.