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 sources of test instability #368

Merged
merged 2 commits into from
Oct 20, 2023
Merged

Conversation

josh-berry
Copy link
Collaborator

@josh-berry josh-berry commented Oct 19, 2023

  • Wait up to 1 second for the server to become healthy before assuming it's failed. After the server starts accepting connections, we were assuming it should be healthy immediately, but that's not always true.

  • Make sure the server finishes shutting down before exiting the CLI, because otherwise it will hold open files, and this intermittently breaks tests on Windows (which, unlike POSIXy OSes, will refuse to delete open files). Closes [Bug] Flaky test: TestCreateDataDirectory_ExistingDirectory #338. I hope.

@josh-berry josh-berry force-pushed the josh/proper-server-shutdown branch from 52aaf37 to 37f2efd Compare October 19, 2023 22:45
@josh-berry josh-berry changed the title Properly shutdown the server before exiting Fix sources of test instability Oct 19, 2023
@josh-berry josh-berry force-pushed the josh/proper-server-shutdown branch from 37f2efd to cf13947 Compare October 19, 2023 22:51
@josh-berry
Copy link
Collaborator Author

josh-berry commented Oct 19, 2023

Manual testing:

  • Verify temporal server start-dev continues to work as expected (and does not immediately shut down the server).
  • Re-run all the GitHub actions a few times to be sure the test flakiness has actually gone away. (Not all of it was reproducible on either of my local machines.)

- Wait up to 10 seconds for the server to become healthy before assuming
  it's failed. After the server starts accepting connections, we were
  assuming it should be healthy immediately, but that's not always true.

- Make sure the server finishes shutting down before exiting the CLI,
  because otherwise it will hold open files, and this intermittently
  breaks tests on Windows (which, unlike POSIXy OSes, will refuse to
  delete open files).  Closes #338.  I hope.
@josh-berry josh-berry force-pushed the josh/proper-server-shutdown branch from cf13947 to b35714a Compare October 19, 2023 23:06
@josh-berry josh-berry marked this pull request as ready for review October 19, 2023 23:06
@josh-berry josh-berry requested a review from a team October 19, 2023 23:06
app/app_test.go Show resolved Hide resolved
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.

[Bug] Flaky test: TestCreateDataDirectory_ExistingDirectory
3 participants