Skip to content

spec_helper: fix and improve retry logic.#10439

Merged
MikeMcQuaid merged 4 commits intoHomebrew:masterfrom
MikeMcQuaid:fix_retry
Jan 27, 2021
Merged

spec_helper: fix and improve retry logic.#10439
MikeMcQuaid merged 4 commits intoHomebrew:masterfrom
MikeMcQuaid:fix_retry

Conversation

@MikeMcQuaid
Copy link
Copy Markdown
Member

@MikeMcQuaid MikeMcQuaid commented Jan 27, 2021

  • always retry each test at least once (confusingly this means a retry count of 2 rather than 1)
  • always wait at least 1 second between retries
  • set a default retry metadata for integration tests rather than overriding any specified values
  • use example.run rather than example.run_with_retry for integration tests because, confusingly, this avoids having the retry count be half what it should be (because the attempts increases by one for each run_with_retry call)
  • use 4 retries for integration tests with 2**attempts*retry_wait (retry wait now being 2) to actually have more retries than before this commit (but with exponential backoff)

This should generally improve test flakiness in CI but particularly improve the cleanup test flake we've seen recently.

@MikeMcQuaid MikeMcQuaid added the critical Critical change which should be shipped as soon as possible. label Jan 27, 2021
@BrewTestBot
Copy link
Copy Markdown
Contributor

Review period skipped due to critical label.

@MikeMcQuaid MikeMcQuaid enabled auto-merge January 27, 2021 12:24
- always retry each test at least once (confusingly this means a retry
  count of 2 rather than 1)
- always wait at least 1 second between retries
- set a default retry metadata for integration tests rather than
  overriding any specified values
- use `example.run` rather than `example.run_with_retry` for integration
  tests because, confusingly, this avoids having the retry count be
  half what it should be (because the attempts increases by one for
  each `run_with_retry` call)
- use 4 retries for integration tests with 2**attempts*retry_wait
  (retry wait now being 2) to actually have more retries than before
  this commit (but with exponential backoff)

This should generally improve test flakiness in CI but particularly
improve the cleanup test flake we've seen recently.
The `VERBOSE_TESTS` variable was from cask and never gets set (and
is unset by `bin/brew`). Replace it with `HOMEBREW_VERBOSE_TESTS` and
set it by `--verbose` or `--debug`.

While we're here, remove an unneeded `VERBOSE` delete (as it's already
done by `bin/brew`).
It's annoying to have these autoremoved.
Let's see if this makes the test more reliable.
@MikeMcQuaid MikeMcQuaid disabled auto-merge January 27, 2021 19:37
@MikeMcQuaid MikeMcQuaid merged commit a83848d into Homebrew:master Jan 27, 2021
@MikeMcQuaid MikeMcQuaid deleted the fix_retry branch January 27, 2021 19:37
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Feb 27, 2021
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Feb 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

critical Critical change which should be shipped as soon as possible. outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants