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

No conflicts as of last run.

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