Skip to content

fix(scaletest): fix flake in Test_Runner/Cleanup #10252

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

Merged
merged 7 commits into from
Oct 16, 2023

Conversation

johnstcn
Copy link
Member

@johnstcn johnstcn commented Oct 13, 2023

Fixes #10240

From the linked issue:

  • We get to the require.Eventually that asserts that the build was canceled
  • This times out after 10 seconds
  • It appears that the build job never got canceled after 10 seconds
  • It also appears that a second build never got created
  • We unfortunately appear to be missing cleanup logs in the output, so hard to troubleshoot further.

Based on the above, it looks like a race between runner.Cleanup doing its thing and the require.Eventually asserting that cleanup happened. I don't see a simple way to synchronize between these two actions, so to reduce the likelihood of this happening in future and hoepfully aid future troubleshooting:

  • Added a check that the build is running before we cancel it
  • Added cleanup output to logs (TODO done)
  • Added some checks to the test cleanup logs

I wasn't able to recrate the original failure after running the test 100 times with DB=ci and -race, so while not 100% confident this completely fixes the flake, I'm hopeful at least.

@johnstcn johnstcn self-assigned this Oct 13, 2023
@johnstcn johnstcn requested a review from spikecurtis October 13, 2023 11:30
@johnstcn johnstcn changed the title fix(scaletest): address flake in Test_Runner/Cleanup fix(scaletest): fix flake in Test_Runner/Cleanup Oct 13, 2023
Copy link
Contributor

@spikecurtis spikecurtis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nits to take care of, but I don't need to re-review

@johnstcn johnstcn merged commit dd86100 into main Oct 16, 2023
@johnstcn johnstcn deleted the cj/flake/test-runner-cleanup-pending-build branch October 16, 2023 11:37
@github-actions github-actions bot locked and limited conversation to collaborators Oct 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test flake: scaletest/createworkspaces Test_Runner/CleanupPendingBuild
2 participants