ci: Reduce use of bash -c #28954

1. 
   maflcko commented at 12:20 pm on November 28, 2023: member

   It is confusing to treat commands as a single string. This change is also required to support paths and strings with spaces in them in the future.
2. 
   DrahtBot commented at 12:20 pm on November 28, 2023: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	vasild, RandyMcMillan

   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:

   • #28736 (ci: run test_bitcoin with DEBUG_LOG_OUT in RUN_UNIT_TESTS_SEQUENTIAL by vasild)

   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.

3. DrahtBot added the label Tests on Nov 28, 2023
4. ci: Rename test script to 03_test_script.sh fafcee4874
5. maflcko force-pushed on Nov 28, 2023
6. DrahtBot added the label CI failed on Nov 28, 2023
7. maflcko force-pushed on Nov 28, 2023
8. maflcko force-pushed on Nov 28, 2023
9. maflcko force-pushed on Nov 28, 2023
10. DrahtBot removed the label CI failed on Nov 29, 2023
11. in ci/test/03_test_script.sh:175 in fad0165de8 outdated

    173 fi
    174 
    175 if [ "$RUN_FUNCTIONAL_TESTS" = "true" ]; then
    176-  bash -c "LD_LIBRARY_PATH=${DEPENDS_DIR}/${HOST}/lib ${TEST_RUNNER_ENV} test/functional/test_runner.py --ci $MAKEJOBS --tmpdirprefix ${BASE_SCRATCH_DIR}/test_runner/ --ansi --combinedlogslen=99999999 --timeout-factor=${TEST_RUNNER_TIMEOUT_FACTOR} ${TEST_RUNNER_EXTRA} --quiet --failfast"
    177+  # shellcheck disable=SC2086
    178+  LD_LIBRARY_PATH="${DEPENDS_DIR}/${HOST}/lib" test/functional/test_runner.py --ci "${MAKEJOBS}" --tmpdirprefix "${BASE_SCRATCH_DIR}"/test_runner/ --ansi --combinedlogslen=99999999 --timeout-factor="${TEST_RUNNER_TIMEOUT_FACTOR}" ${TEST_RUNNER_EXTRA} --quiet --failfast
    

    ---

    ---

    

    vasild commented at 2:02 pm on November 29, 2023:

    nit: some inconsistency wrt "${FOO}"/bar vs "${FOO}/bar" which to my understanding are the same thing.

    ---

    

    maflcko commented at 2:13 pm on November 29, 2023:

    Yes, it is the same, so going to leave as is for now.
12. in ci/test/00_setup_env_i686_multiprocess.sh:17 in fad0165de8 outdated

    13@@ -14,4 +14,4 @@ export DEP_OPTS="DEBUG=1 MULTIPROCESS=1"
    14 export GOAL="install"
    15 export BITCOIN_CONFIG="--enable-debug CC='clang -m32' CXX='clang++ -m32' \
    16 LDFLAGS='--rtlib=compiler-rt -lgcc_s' CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE'"
    17-export TEST_RUNNER_ENV="BITCOIND=bitcoin-node"
    

    ---

    ---

    

    vasild commented at 2:04 pm on November 29, 2023:

    ci/test/00_setup_env.sh still contains export TEST_RUNNER_ENV=${TEST_RUNNER_ENV:-} which can be dropped as well coz it is the only remaining reference to TEST_RUNNER_ENV.

    ---

    

    maflcko commented at 2:13 pm on November 29, 2023:

    Nice find. Removed the stale export
13. vasild approved
14. 
    vasild commented at 2:06 pm on November 29, 2023: contributor

    ACK fad0165de85e1c0a39e511a619e9efcee69ac581 modulo the removal of the now stale reference to TEST_RUNNER_ENV.
15. ci: Reduce use of bash -c

    It is confusing to treat commands as a single string. This change is
    also required to support paths and strings with spaces in them in the
    future.
    
    This requires replacing TEST_RUNNER_ENV with a global export, because it
    no longer works. See:
    
    ```bash
    $ export ENV="A=1" && $ENV ls
    bash: A=1: command not found...
    ```
    
    Or in the CI task:
    
    + DIR_UNIT_TEST_DATA=/ci_container_base/ci/scratch/qa-assets/unit_test_data/
    + LD_LIBRARY_PATH=/ci_container_base/depends/i686-pc-linux-gnu/lib
    + BITCOIND=bitcoin-node make -j10 check VERBOSE=1
    /ci_container_base/ci/test/03_test_script.sh: line 166: BITCOIND=bitcoin-node: command not found
    
    https://github.com/bitcoin/bitcoin/pull/28954/checks?check_run_id=19096858944
    https://cirrus-ci.com/task/6718317604372480

    fad82fea2b
16. maflcko force-pushed on Nov 29, 2023
17. 
    maflcko commented at 2:14 pm on November 29, 2023: member

    force pushed to remove a leftover stale export
18. vasild approved
19. 
    vasild commented at 2:18 pm on November 29, 2023: contributor

    ACK fad82fea2bef88c9f11e25ca43c7886a2b9b5da2
20. 
    RandyMcMillan commented at 3:33 pm on November 29, 2023: contributor

    utACK fad82fea2bef88c9f11e25ca43c7886a2b9b5da2
21. fanquake merged this on Nov 30, 2023
22. fanquake closed this on Nov 30, 2023

    ---

23. maflcko deleted the branch on Nov 30, 2023

Contributors
maflcko DrahtBot vasild RandyMcMillan

Labels
Tests