test-each-commit
#34383
test-each-commit currently tests only a limited number of ancestor commits in a PR, so failures introduced deeper in the commit stack might be missed.
Remove the max-count limit so test-each-commit runs the full build + unit + functional test flow on every non-head PR commit, while keeping the PR tip excluded because it is already covered by the normal CI jobs.
Note: this PR has gone through a few other versions, the latest one just extends the existing job.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34383.
See the guideline for information on the review process. A summary of reviews will appear here.
Reviewers, this pull request conflicts with the following ones:
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.
🚧 At least one of the CI tasks failed.
Task lint: https://github.com/bitcoin/bitcoin/actions/runs/21265870531/job/61204792016
LLM reason (✨ experimental): Lint failure: a file is not executable (ci-compile-each-commit-exec.py has 644 permission).
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
ci-test-each-commit-exec.py
unrelated note: I think this script is missing the functional test runner --failfast option. A failure will lead to a massive log output with all failures collected, which is hard to read and debug. It would be better to just print the first failure.
More generally, I wonder if it wouldn’t be easier to just run it on all commits. I guess the timeout could be raised to the max (6 hours), possibly using a cirrus runner.
If a pull request doesn’t finish after 6 hours on a 16 core machine, then it is probably too large and should be split up?
If it isn’t possible to split up, one or two reviewers can run the script locally?
Ah, with “it” I meant “pull request”, not “the ci task here”.
If a pull request doesn’t finish after 6 hours on a 16 core machine, then it is probably too large and should be split up?
Was there ever a pull request in history that wouldn’t compile and test all commits in 6 hours on a modern 16 core machine?
Was there ever a pull request in history that wouldn’t compile and test all commits in 6 hours on a modern 16 core machine?
let’s find out
the leveldb and crc32 updates don’t always compile, but I couldn’t find any other
edit: e.g. first commit of #33641 fails with util/env_posix.cc:857:24: error: no template named 'is_standard_layout_v' in namespace 'std'; did you mean 'is_standard_layout'? And 9a5d29711afcdc4609da4786673758e641958bb4 also fails with old cmake requirement.
the leveldb and crc32 updates don’t always compile, but I couldn’t find any other
edit: e.g. first commit of #33641 fails with
util/env_posix.cc:857:24: error: no template named 'is_standard_layout_v' in namespace 'std'; did you mean 'is_standard_layout'? And 9a5d297 also fails with old cmake requirement.
That pull request has a single commit, so it seems impossible for it to time out. Also, if it timed-out the current CI task would time out as well.
If you want to actually test this, my recommendation would be to only look at pulls with more than 6 commits.
you mean that f21162d was already skipped by CI?
The commit is both a child of a merge commit and also a subtree commit of a different repo. You can also check the behavior here https://github.com/bitcoin/bitcoin/actions/runs/21197620806/job/60976836032?pr=34363, which is another subtree bump.
Extend `test-each-commit` to run on every non-head pull request commit.
The PR tip is excluded because it is already covered by other CI jobs.
Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>test-each-commit job.