Skip to content

Conversation

mzumsande
Copy link
Contributor

Fixes #23990

After #23799, the --failfast option in the test runner for the functional tests stopped working, because a second outer loop was introduced, which would have needed a break too for the test runner to fail immediately. This also led to the errors reported in #23990.

This provides a straightforward fix for that.
There is also #23995 which is a larger refactor, but that hasn't been updated in a while to fix the failfast issue.

@DrahtBot DrahtBot added the Tests label Jan 28, 2022
@DrahtBot
Copy link
Contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #23995 (Simplify test_runner.py a bit by sipa)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@pg156
Copy link

pg156 commented Feb 5, 2022

Tested ACK a036358. I agree adding the all_passed flag to break out of the outer loop when needed makes sense. The "failfast" option works after this change.

@maflcko maflcko merged commit 9392e13 into bitcoin:master Feb 7, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 7, 2022
…nner

a036358 test: Repair failfast option for test runner (Martin Zumsande)

Pull request description:

  Fixes bitcoin#23990

  After bitcoin#23799, the `--failfast` option in the test runner for the functional tests stopped working, because a second outer loop was introduced, which would have needed a `break` too for the test runner to fail immediately. This also led to the errors reported in bitcoin#23990.

  This provides a straightforward fix for that.
  There is also bitcoin#23995 which is a larger refactor, but that hasn't been updated in a while to fix the failfast issue.

ACKs for top commit:
  pg156:
    Tested ACK a036358. I agree adding the `all_passed` flag to break out of the outer loop when needed makes sense. The "failfast" option works after this change.

Tree-SHA512: 3e2f775e36c13d180d32a05cd1cfe0883274e8615cdbbd4e069a9899e9b9ea1091066cf085e93f1c5326bd8ecc6ff524e0dad7c638f60dfdb169fefcdb26ee52
@katesalazar
Copy link
Contributor

katesalazar commented Mar 9, 2022 via email

@bitcoin bitcoin locked and limited conversation to collaborators Mar 9, 2023
@mzumsande mzumsande deleted the 202201_testrunner_fix branch June 23, 2023 18:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test_runner.py issue
5 participants