scripted-diff: [test] replace assert with Assert - #36074
Conversation
DrahtBot
changed the title
|
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/36074. ReviewsSee the guideline and AI policy for information on the review process. ConflictsReviewers, 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. LLM Linter (✨ experimental)Possible typos and grammar issues:
Possible places where named args for integral literals may be used (e.g.
2026-08-26 11:29:19 |
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
|
Hmm, I guess I guess that means we could add an explicit |
The project has a compile error when compiled without assertions in
util/check.h. Thus, util/check.h should be included for all assertions.
So do that with a scripted-diff for Assert and assert, and remove the
cassert include, which is exported from util/check.h.
-BEGIN VERIFY SCRIPT-
# Select all test .cpp and .h files
paths=(
'src/test/'
'src/bench/'
'src/wallet/test/'
'src/qt/test/'
':(exclude)src/bench/nanobench.h'
)
# Add the util/check.h includes
for f in $(git grep -l --extended-regexp "\<(a|A)ssert\(" -- "${paths[@]}"); do
if ! grep --quiet "util/check.h" "$f"; then
if [[ "$f" == *.cpp ]]; then
line=5
else
line=8
fi
sed --in-place "${line}i#include <util/check.h>" "$f"
fi
done
# Remove cassert includes
for f in $(git grep -l '<cassert>' -- "${paths[@]}"); do
sed --in-place '/^#include <cassert>$/d' "$f"
done
-END VERIFY SCRIPT-
This can be reproduced by checking out the previous commit and running: git show -U0 | ./contrib/devtools/clang-format-diff.py -p1 -i -v
-BEGIN VERIFY SCRIPT-
sed -i --regexp-extended 's/\<assert\(/Assert(/g' $(git grep -l --extended-regexp '\<assert\(' -- \
src/test/ \
src/bench/ \
src/wallet/test/ \
src/qt/test/ \
":(exclude)src/bench/nanobench.h" \
)
-END VERIFY SCRIPT-
fa7f786 to
3440553
Compare
| Assert(!mini_miner.IsReadyToCalculate()); | ||
| } | ||
| // Overlapping ancestry across multiple outpoints can only reduce the total bump fee. | ||
| assert (sum_fees >= *total_bumpfee); |
There was a problem hiding this comment.
This one doesn't match \<assert\( because of the space, and it's the only one left in the swept paths. lint_c_assert uses the same regex, so it doesn't catch it either. I added assert (1); to a covered file and the linter still passes. How about \<assert\s*\(?
| "src/test/", | ||
| "src/bench/", | ||
| "src/wallet/test/", | ||
| "src/qt/test/", |
There was a problem hiding this comment.
src/ipc/test/ is left out here, and src/ipc/test/fuzz/ipc.cpp has six assert(. lint-tests.py already counts that directory as part of the test suite. Is this intentional?
3 participants
In test code,
assertis used. This is perfectly fine, but sometimes confusion arises, when it is unclear whetherNDEBUGcan disable the assertions, or whether to useassertorAssert.Avoid that confusion in test code with a scripted replacement and a small linter to enforce it going forward.
Scope: This change is only about test code (bench, fuzz, unit), other code can be done later, if there is need to.