Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix flake: https://ci.korifi.cf-app.com/teams/main/pipelines/main/jobs/run-tests-periodic/builds/13588 #3011

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

georgethebeatle
Copy link
Member

Is there a related GitHub Issue?

No

What is this change about?

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:

Does this PR introduce a breaking change?

No

Acceptance Steps

Green tests

Tag your pair, your PM, and/or team

@danail-branekov

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
@danail-branekov danail-branekov merged commit 9d0841e into main Nov 28, 2023
@danail-branekov danail-branekov deleted the wip-package-clean-flake branch November 28, 2023 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants