test: drop run_all_tests.sh and document how to run scenario tests - #3577
Merged
Conversation
The scenario tests moved from nose to pytest in d6dee83, but run_all_tests.sh kept invoking each test file as a plain script. The test files have no __main__ block, so every invocation imported the module, ran nothing and exited 0. For two months the script reported "all tests passed successfully" without running a single test, and nobody noticed. The same breakage sat in the README instructions for running one test. It was never a full run in the first place. It covered 13 of the 30 test files, and it could not prepare the host for two of those 13. bgp_unnumbered_test.py needs IPv6 in Docker and a daemon restart, which would kill the other twelve tests the script had already started in parallel, and tcp_md5_test.py needs the vrf module. Three further test files need a second image built on an older Quagga. Fixing the script means keeping a second test list beside ci.yml, and drift between the two is what broke it. pytest already runs one file or the whole directory with no list to maintain, so delete the script instead. Document in the README what until now only ci.yml knew: the gobgp-oq image, the Docker IPv6 setup, and the vrf module. Assisted-by: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The scenario tests moved from nose to pytest in d6dee83, but run_all_tests.sh kept invoking each test file as a plain script. The test files have no main block, so every invocation imported the module, ran nothing and exited 0. For two months the script reported "all tests passed successfully" without running a single test, and nobody noticed. The same breakage sat in the README instructions for running one test.
It was never a full run in the first place. It covered 13 of the 30 test files, and it could not prepare the host for two of those 13. bgp_unnumbered_test.py needs IPv6 in Docker and a daemon restart, which would kill the other twelve tests the script had already started in parallel, and tcp_md5_test.py needs the vrf module. Three further test files need a second image built on an older Quagga. Fixing the script means keeping a second test list beside ci.yml, and drift between the two is what broke it. pytest already runs one file or the whole directory with no list to maintain, so delete the script instead.
Document in the README what until now only ci.yml knew: the gobgp-oq image, the Docker IPv6 setup, and the vrf module.
Assisted-by: Claude Opus 5 noreply@anthropic.com