test: Move `script_assets_tests` into its own suite

hebasto commented at 7:28 pm on December 27, 2024: member

This PR ensures that the script_assets_tests test case is explicitly reported as “Skipped” when it is not run, making it clearer when running the test suite with ctest:

on the master branch @ 9355578a77978a0c2f189bd7315a2883142d8119:

 0$ env -u DIR_UNIT_TEST_DATA ctest --test-dir build -j 16 -R "^script_"
 1Internal ctest changing into directory: /home/hebasto/git/bitcoin/build
 2Test project /home/hebasto/git/bitcoin/build
 3    Start 87: script_tests
 4    Start 83: script_p2sh_tests
 5    Start 85: script_segwit_tests
 6    Start 86: script_standard_tests
 7    Start 84: script_parse_tests
 81/5 Test [#84](/bitcoin-bitcoin/84/): script_parse_tests ...............   Passed    0.11 sec
 92/5 Test [#86](/bitcoin-bitcoin/86/): script_standard_tests ............   Passed    0.11 sec
103/5 Test [#85](/bitcoin-bitcoin/85/): script_segwit_tests ..............   Passed    0.12 sec
114/5 Test [#83](/bitcoin-bitcoin/83/): script_p2sh_tests ................   Passed    0.12 sec
125/5 Test [#87](/bitcoin-bitcoin/87/): script_tests .....................   Passed    0.36 sec
13
14100% tests passed, 0 tests failed out of 5
15
16Total Test time (real) =   0.37 sec

with this PR:

 0$ env -u DIR_UNIT_TEST_DATA ctest --test-dir build -j 16 -R "^script_"
 1Internal ctest changing into directory: /home/hebasto/git/bitcoin/build
 2Test project /home/hebasto/git/bitcoin/build
 3    Start 83: script_assets_tests
 4    Start 88: script_tests
 5    Start 84: script_p2sh_tests
 6    Start 86: script_segwit_tests
 7    Start 87: script_standard_tests
 8    Start 85: script_parse_tests
 91/6 Test [#85](/bitcoin-bitcoin/85/): script_parse_tests ...............   Passed    0.11 sec
102/6 Test [#83](/bitcoin-bitcoin/83/): script_assets_tests ..............***Skipped   0.12 sec
113/6 Test [#86](/bitcoin-bitcoin/86/): script_segwit_tests ..............   Passed    0.11 sec
124/6 Test [#87](/bitcoin-bitcoin/87/): script_standard_tests ............   Passed    0.11 sec
135/6 Test [#84](/bitcoin-bitcoin/84/): script_p2sh_tests ................   Passed    0.12 sec
146/6 Test [#88](/bitcoin-bitcoin/88/): script_tests .....................   Passed    0.36 sec
15
16100% tests passed, 0 tests failed out of 6
17
18Total Test time (real) =   0.37 sec
19
20The following tests did not run:
21	 83 - script_assets_tests (Skipped)
22$ env DIR_UNIT_TEST_DATA=/home/hebasto/git/bitcoin/qa-assets/unit_test_data ctest --test-dir build -j 16 -R "^script_"
23Internal ctest changing into directory: /home/hebasto/git/bitcoin/build
24Test project /home/hebasto/git/bitcoin/build
25    Start 83: script_assets_tests
26    Start 88: script_tests
27    Start 84: script_p2sh_tests
28    Start 86: script_segwit_tests
29    Start 87: script_standard_tests
30    Start 85: script_parse_tests
311/6 Test [#85](/bitcoin-bitcoin/85/): script_parse_tests ...............   Passed    0.11 sec
322/6 Test [#87](/bitcoin-bitcoin/87/): script_standard_tests ............   Passed    0.11 sec
333/6 Test [#86](/bitcoin-bitcoin/86/): script_segwit_tests ..............   Passed    0.11 sec
344/6 Test [#84](/bitcoin-bitcoin/84/): script_p2sh_tests ................   Passed    0.12 sec
355/6 Test [#88](/bitcoin-bitcoin/88/): script_tests .....................   Passed    0.35 sec
366/6 Test [#83](/bitcoin-bitcoin/83/): script_assets_tests ..............   Passed    1.58 sec
37
38100% tests passed, 0 tests failed out of 6
39
40Total Test time (real) =   1.58 sec

hebasto added the label Tests on Dec 27, 2024

DrahtBot commented at 7:28 pm on December 27, 2024: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31576.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	maflcko
Stale ACK	yuvicc

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32473 (Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts 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.

hebasto marked this as a draft on Dec 27, 2024

hebasto force-pushed on Dec 27, 2024

hebasto marked this as ready for review on Dec 27, 2024

hebasto commented at 3:01 pm on January 7, 2025: member

See the use case in https://github.com/hebasto/bitcoin-core-nightly/pull/27.

hebasto commented at 9:54 am on February 10, 2025: member

Friendly ping @maflcko ;)

maflcko commented at 5:17 pm on February 25, 2025: member

Seems fine. No strong opinion.

yuvicc commented at 7:27 pm on May 17, 2025: contributor

tACK 0859fc22f0baf00259cf964f9d39d36fba6cd291

making skipped tests explicitly appear as ‘Skipped’ rather than silently omitting

 0    Start 89: script_tests
 1    Start 85: script_p2sh_tests
 2    Start 87: script_segwit_tests
 3    Start 88: script_standard_tests
 4    Start 86: script_parse_tests
 5    Start 84: script_assets_tests
 61/6 Test [#86](/bitcoin-bitcoin/86/): script_parse_tests ...............   Passed    0.16 sec
 72/6 Test [#84](/bitcoin-bitcoin/84/): script_assets_tests ..............***Skipped   0.16 sec
 83/6 Test [#88](/bitcoin-bitcoin/88/): script_standard_tests ............   Passed    0.17 sec
 94/6 Test [#87](/bitcoin-bitcoin/87/): script_segwit_tests ..............   Passed    0.17 sec
105/6 Test [#85](/bitcoin-bitcoin/85/): script_p2sh_tests ................   Passed    0.17 sec
116/6 Test [#89](/bitcoin-bitcoin/89/): script_tests .....................   Passed    0.37 sec
12
13100% tests passed, 0 tests failed out of 6
14
15Total Test time (real) =   0.38 sec
16
17The following tests did not run:
18	 84 - script_assets_tests (Skipped)

in src/test/script_assets_tests.cpp:33 in 0859fc22f0 outdated

28+unsigned int ParseScriptFlags(std::string strFlags);
29+
30+BOOST_AUTO_TEST_SUITE(script_assets_tests)
31+
32+/* Wrapper around ProduceSignature to combine two scriptsigs */
33+SignatureData CombineSignatures(const CTxOut& txout, const CMutableTransaction& tx, const SignatureData& scriptSig1, const SignatureData& scriptSig2)

maflcko commented at 8:01 am on June 20, 2025:

what is this for?

hebasto commented at 9:10 am on June 20, 2025:

Thanks! Dropped.

maflcko approved

maflcko commented at 8:04 am on June 20, 2025: member

makes sense to be explicit here to avoid confusion

lgtm ACK 0859fc22f0baf00259cf964f9d39d36fba6cd291 🐽

Signature:

0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: lgtm ACK 0859fc22f0baf00259cf964f9d39d36fba6cd291 🐽
3oguvM7vjti8dmtabMmgzCKuxp+/7OOHqZVEMvjj+RjTVxfTJLoC+HJhdnAQ11Y5a2tsJI5smtPebrAKwdUAKBA==

test: Move `script_assets_tests` into its own suite

This change ensures that the `script_assets_tests` test case is
explicitly reported as "Skipped" when it is not run, making it clearer
when running the test suite with `ctest`.

c40dbbbf77

hebasto force-pushed on Jun 20, 2025

hebasto commented at 9:11 am on June 20, 2025: member

Addressed feedback from @maflcko.

maflcko commented at 9:42 am on June 20, 2025: member

only change is dropping dead code

re-ACK c40dbbbf77078bb86a652ca151b472e7bef61ae0 👈

Signature:

0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: re-ACK c40dbbbf77078bb86a652ca151b472e7bef61ae0 👈
3NetYDFd/QmbGErA2ugrrDGOZgiUEvhYtaPbG9oaznL6YlEGo5+XntdHOPsxEkTl4G1PYEoc+dEs9jOccdWLJDA==

DrahtBot requested review from yuvicc on Jun 20, 2025

test: Move script_assets_tests into its own suite #31576

Code Coverage & Benchmarks

Reviews

Conflicts

test: Move `script_assets_tests` into its own suite #31576