ci: Remove redundant RUN_UNIT_TESTS_SEQUENTIAL #33136
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33136. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsNo conflicts as of last run. |
| @@ -374,7 +374,7 @@ jobs: | |||
| - name: Run unit tests | |||
| # Can't use ctest here like other jobs as we don't have a CMake build tree. | |||
| run: | | |||
| ./bin/test_bitcoin.exe -l test_suite | |||
| ./bin/test_bitcoin.exe -l test_suite # Intentionally run sequentially here, to catch test case failures caused by dirty global state from prior test cases. | |||
There was a problem hiding this comment.
to catch test case failures caused by dirty global state from prior test cases.
If the aim is to catch dirty state, should we run it in a job with the most unit tests? Windows cross-build is going to run less than Linux.
There was a problem hiding this comment.
I think it is always possible to miss unit tests that are not compiled. For the Windows ones, it should only be a few about windows paths, fork(), and socketpair, which seems fine to skip.
If there is need to run it in another task, it can be done as a follow-up?
faaeaba to
fa40da9
Compare
|
(trivial rebase) |
fa40da9 to
faed66a
Compare
faed66a to
fae610d
Compare
RUN_UNIT_TESTS_SEQUENTIALis useful to detect cases where global state is left dirty in the test process and leads to subsequent unit test cases failing. However, one CI task is sufficient to catch this.As there already is one, add docs there and remove this env var (and extra logic).