test: Move script_assets_tests into its own suite #31576

pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:241227-test-assets changing 3 files +181 −141
  1. 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
    
  2. hebasto added the label Tests on Dec 27, 2024
  3. 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.

  4. hebasto marked this as a draft on Dec 27, 2024
  5. hebasto force-pushed on Dec 27, 2024
  6. hebasto marked this as ready for review on Dec 27, 2024
  7. hebasto commented at 3:01 pm on January 7, 2025: member
  8. hebasto commented at 9:54 am on February 10, 2025: member
    Friendly ping @maflcko ;)
  9. maflcko commented at 5:17 pm on February 25, 2025: member
    Seems fine. No strong opinion.
  10. 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)
    
  11. 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.
  12. maflcko approved
  13. 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==
    
  14. 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
  15. hebasto force-pushed on Jun 20, 2025
  16. hebasto commented at 9:11 am on June 20, 2025: member
    Addressed feedback from @maflcko.
  17. 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==
    
  18. DrahtBot requested review from yuvicc on Jun 20, 2025

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-07-12 09:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me