ci: Download script_assets_test.json for Windows CI #34679

pull maflcko wants to merge 4 commits into bitcoin:master from maflcko:2602-win-cross-json changing 5 files +91 −54
  1. maflcko commented at 9:28 am on February 26, 2026: member
    Fixes #34670 by adding a new download_script_assets Python helper and calling it.
  2. DrahtBot renamed this:
    ci: Download script_assets_test.json for Windows CI
    ci: Download script_assets_test.json for Windows CI
    on Feb 26, 2026
  3. DrahtBot added the label Tests on Feb 26, 2026
  4. DrahtBot commented at 9:29 am on February 26, 2026: contributor

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

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK hebasto, janb84, hodlinator

    If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #34614 (ci: Put space and non-ASCII char in scratch dir by maflcko)

    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.

  5. DrahtBot added the label CI failed on Feb 26, 2026
  6. test: [doc] Remove outdated comment 
    The curl requirement has long been removed, when windows support was
added to the script. Also, the build support has long been dropped,
since the project switched from autotools to cmake.
    fa0cc1c5a4
  7. maflcko force-pushed on Feb 26, 2026
  8. DrahtBot removed the label CI failed on Feb 26, 2026
  9. maflcko force-pushed on Feb 26, 2026
  10. in test/download_utils.py:1 in fa76dc11e8 outdated
    janb84 commented at 2:31 pm on February 26, 2026:
    For now this is only download-utils, maybe download-utils.py would be a more descriptive filename (But also limiting). Do you see any near term code moves to this util file ?
    maflcko commented at 2:58 pm on February 26, 2026:

    Good question. Probably not much that could move here, as this is meant to be used for shared ci+test stuff. Anything specific to ci or tests can live in the ./ci or ./test folder.

    So renaming could make sense.

  11. janb84 commented at 2:47 pm on February 26, 2026: contributor

    Concept ACK fa76dc11e8534af9658781d89f5ea4df4d4bf574

    Code review, and validated that it fixes #34670

    Master fails: https://github.com/bitcoin/bitcoin/actions/runs/22438260770/job/64989748768#step:8:1041

    0./test/script_assets_tests.cpp(156): warning: in "script_assets_tests/script_assets_test": Variable DIR_UNIT_TEST_DATA unset, skipping script_assets_test

    This PR, succeeds:

    https://github.com/bitcoin/bitcoin/actions/runs/22440340500/job/64981475935?pr=34679#step:9:1042

    0./test/script_assets_tests.cpp(30): Entering test suite "script_assets_tests"
1./test/script_assets_tests.cpp(149): Entering test case "script_assets_test"
2./test/script_assets_tests.cpp(149): Leaving test case "script_assets_test"; testing time: 1311764us
3./test/script_assets_tests.cpp(30): Leaving test suite "script_assets_tests"; testing time: 1311822us

    the changed prev_releases also still works: https://github.com/bitcoin/bitcoin/actions/runs/22440340500/job/64981475935?pr=34679#step:8:233

  12. maflcko force-pushed on Feb 26, 2026
  13. test: move-only download_from_url to stand-alone util file 
    Can be reviewed via --color-moved=dimmed-zebra
    faf96286ce
  14. test: Move Fetching-print to download_from_url util 
    This does not change any behavior.
    7777a13306
  15. maflcko force-pushed on Feb 26, 2026
  16. DrahtBot added the label CI failed on Feb 26, 2026
  17. maflcko force-pushed on Feb 26, 2026
  18. hebasto commented at 4:23 pm on February 26, 2026: member
    Concept ACK.
  19. in .github/ci-windows.py:121 in fa0f8e0732 outdated 
    123@@ -118,13 +124,14 @@ def prepare_tests(ci_type):
124         print("Using qa-assets repo from commit ...")
125         run(["git", "-C", repo_dir, "log", "-1"])
126 
127-
    hebasto commented at 4:36 pm on February 26, 2026:
    nit: Keep two empty lines between functions?
  20. hebasto approved
  21. hebasto commented at 4:37 pm on February 26, 2026: member
    ACK fa0f8e0732dd1b9874d0a1f8b2b4917c87a821fa.
  22. DrahtBot requested review from janb84 on Feb 26, 2026
  23. ci: Download script_assets_test.json for Windows CI fa7612f253
  24. maflcko force-pushed on Feb 26, 2026
  25. hebasto approved
  26. hebasto commented at 5:15 pm on February 26, 2026: member
    re-ACK fa7612f2536b0ef47334105cf657879cdcc3a579.
  27. DrahtBot removed the label CI failed on Feb 26, 2026
  28. janb84 commented at 7:17 pm on February 26, 2026: contributor

    re ACK fa7612f2536b0ef47334105cf657879cdcc3a579

    thanks for taking my suggestion!

  29. fanquake commented at 4:59 pm on March 3, 2026: member
    cc @hodlinator
  30. in test/download_utils.py:52 in fa7612f253 
    47+            raise RuntimeError(f"Download incomplete: expected {total_size} bytes, got {progress_bytes} bytes")
48+
49+    print('\n', flush=True, end="") # Flush to avoid error output on the same line.
50+
51+
52+def download_script_assets(script_assets_dir):
    hodlinator commented at 8:26 pm on March 3, 2026:
    How about also changing 03_test_script.sh to use this functionality instead of keeping it duplicated?
    maflcko commented at 8:41 pm on March 3, 2026:

    Happy to push a patch. The CI script is still written in Bash, so I guess this requires something like:

    0python3 - <<EOF
1import sys
2from pathlib import Path
3
4sys.path.append(str(Path("$BASE_ROOT_DIR") / "test"))
5from download_utils import download_script_assets
6
7download_script_assets()
8EOF
    hodlinator commented at 9:43 pm on March 3, 2026:

    Was thinking more along the lines of

     0diff --git a/ci/test/03_test_script.sh b/ci/test/03_test_script.sh
 1index d81bd0b9ea..da153680d5 100755
 2--- a/ci/test/03_test_script.sh
 3+++ b/ci/test/03_test_script.sh
 4@@ -56,10 +56,7 @@ if [ "$RUN_FUZZ_TESTS" = "true" ]; then
 5   )
 6 elif [ "$RUN_UNIT_TESTS" = "true" ]; then
 7   export DIR_UNIT_TEST_DATA=${DIR_QA_ASSETS}/unit_test_data/
 8-  if [ ! -d "$DIR_UNIT_TEST_DATA" ]; then
 9-    mkdir -p "$DIR_UNIT_TEST_DATA"
10-    ${CI_RETRY_EXE} curl --location --fail https://github.com/bitcoin-core/qa-assets/raw/main/unit_test_data/script_assets_test.json -o "${DIR_UNIT_TEST_DATA}/script_assets_test.json"
11-  fi
12+  python3 "${BASE_ROOT_DIR}/test/download_utils.py" download_script_assets "$DIR_UNIT_TEST_DATA"
13 fi
14 
15 # Make sure default datadir does not exist and is never read by creating a dummy file
16diff --git a/test/download_utils.py b/test/download_utils.py
17index c7fd6cf0c6..87be0c9c47 100644
18--- a/test/download_utils.py
19+++ b/test/download_utils.py
20@@ -7,6 +7,7 @@
21 import sys
22 import time
23 import urllib.request
24+from pathlib import Path
25 
26 
27 def download_from_url(url, archive):
28@@ -63,3 +64,14 @@ def download_script_assets(script_assets_dir):
29             download_from_url(url, script_assets)
30         except Exception as e2:
31             sys.exit(e2)
32+
33+
34+if __name__ == '__main__':
35+    if len(sys.argv) > 1 and sys.argv[1] == download_script_assets.__name__:
36+        if len(sys.argv) != 3:
37+            print(f"Unexpected number of args: {len(sys.argv)}", file=sys.stderr)
38+            sys.exit()
39+        download_script_assets(Path(sys.argv[2]))
40+    else:
41+        print(f"Unexpected args: {sys.argv}", file=sys.stderr)
42+        sys.exit()

    Ran locally with native_asan env and couldn’t spot the any “Fetching…” output in the console, but the json file had been downloaded.

  31. hodlinator approved
  32. hodlinator commented at 8:53 pm on March 3, 2026: contributor

    utACK fa7612f2536b0ef47334105cf657879cdcc3a579

    Thanks for fixing the issue! Extracting download_utils.py is a nice approach.

    CI Output

    Windows native certainly seems to be doing something now:

    0 95/155 Test  [#98](/bitcoin-bitcoin/98/): scriptnum_tests ......................   Passed    0.17 sec
1        Start  99: serfloat_tests
2 96/155 Test  [#92](/bitcoin-bitcoin/92/): script_assets_tests ..................   Passed    4.02 sec

    (https://github.com/bitcoin/bitcoin/actions/runs/22452409131/job/65024351795?pr=34679#step:16:398, download at https://github.com/bitcoin/bitcoin/actions/runs/22452409131/job/65024351795?pr=34679#step:15:210)

    Cross-build also seems to be doing its thing:

    0Fetching: https://github.com/bitcoin-core/qa-assets/raw/main/unit_test_data/script_assets_test.json

    (https://github.com/bitcoin/bitcoin/actions/runs/22452409131/job/65025019000?pr=34679#step:8:366)

    0./test/script_assets_tests.cpp(30): Leaving test suite "script_assets_tests"; testing time: 1333633us

    (https://github.com/bitcoin/bitcoin/actions/runs/22452409131/job/65025019000?pr=34679#step:9:1042)

  33. fanquake merged this on Mar 4, 2026
  34. fanquake closed this on Mar 4, 2026


maflcko DrahtBot janb84 hebasto fanquake hodlinator

Labels
Tests

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: 2026-03-09 09:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me