travis: improve readability of travis.yml and log outputs #13779

1. 
   scravy commented at 10:22 AM on July 27, 2018: contributor

   This PR contains two commits which

   • make the travis.yml file a bit more readable by splitting the environment variables over lines (it's a little bit more verbose but way easier to compare across forks or visually grasp in the first place),
   • use travis:fold to fold the autogen, configure, make, and so forth, steps.

   The END_FOLD shell function preserves the exit code of the statement right before.

2. make .travis/yml a bit more readable 9730d3af31
3. fanquake added the label Tests on Jul 27, 2018
4. fanquake requested review from MarcoFalke on Jul 27, 2018
5. 
   MarcoFalke commented at 10:58 AM on July 27, 2018: member

   ACK c2f3dc0b37d3b8c442d2aa0e92db64fe3b98bedf. OK with me.

6. 
   practicalswift commented at 11:27 AM on July 27, 2018: contributor

   Concept ACK

   Very nice first-time contribution! Keep it coming! :-)

7. 
   DrahtBot commented at 12:04 PM on July 27, 2018: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->Note to reviewers: This pull request conflicts with the following ones:

   • #13515 (travis: Enable Qt build for Windows , i686 and arm Linux by ken2812221)

   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.

8. 
   scravy commented at 12:52 PM on July 27, 2018: contributor

   The build actually failed for one item in the build matrix, specifically the functional tests in rpc_createmultisig ( https://travis-ci.org/bitcoin/bitcoin/jobs/408882695#L3853 ).

   I can't see how this would have been effected by the change to the travis file, so I'm wondering if it's like this in master and whether I should rebase.

   I read in the contribution guidelines "sometimes you might be asked to rebase by a reviewer" – is rebasing in general ok? I am wondering as one of the comments refers to a commit in here and rebasing will change it's hash, so technically you're pointing into a fork of my git history which no longer should be valid :-)

9. DrahtBot cross-referenced this on Jul 27, 2018 from issue travis: Enable qt for all jobs by ken2812221
10. 
    ken2812221 commented at 1:00 PM on July 27, 2018: contributor

    utACK c2f3dc0b37d3b8c442d2aa0e92db64fe3b98bedf

11. in .travis.yml:126 in c2f3dc0b37 outdated

     129 | -    - if [ "$RUN_TESTS" = "true" ]; then DOCKER_EXEC test/functional/test_runner.py --combinedlogslen=4000 --coverage --quiet --failfast ${extended}; fi
     130 | +    - BEGIN_FOLD configure; DOCKER_EXEC ./configure --cache-file=../config.cache $BITCOIN_CONFIG_ALL $BITCOIN_CONFIG || ( cat config.log && false); END_FOLD
     131 | +    - BEGIN_FOLD build; DOCKER_EXEC make $MAKEJOBS $GOAL || ( echo "Build failure. Verbose build follows." && DOCKER_EXEC make $GOAL V=1 ; false ); END_FOLD
     132 | +    - BEGIN_FOLD unit-tests; if [ "$RUN_TESTS" = "true" ]; then DOCKER_EXEC LD_LIBRARY_PATH=$TRAVIS_BUILD_DIR/depends/$HOST/lib make $MAKEJOBS check VERBOSE=1; fi; END_FOLD
     133 | +    - BEGIN_FOLD extended-tests; if [ "$TRAVIS_EVENT_TYPE" = "cron" ]; then extended="--extended --exclude feature_pruning,feature_dbcrash"; fi; END_FOLD
     134 | +    - BEGIN_FOLD functional-tests; if [ "$RUN_TESTS" = "true" ]; then DOCKER_EXEC test/functional/test_runner.py --combinedlogslen=4000 --coverage --quiet --failfast ${extended}; fi; END_FOLD
    

    ---

    ---

    

    Empact commented at 2:31 PM on July 27, 2018:

    No point in including the fold if the relevant option is off, right? How about put the BEGIN_FOLD within the ifs?

    ---

    

    scravy commented at 5:37 PM on July 27, 2018:

    Fixed

12. 
    Fuzzbawls commented at 12:13 AM on July 28, 2018: contributor

    Concept ACK

    Improved readability is always nice :)

13. 
    Empact commented at 3:57 PM on July 28, 2018: member

    I notice extended-tests just sets up config for test_runner - it doesn't need a fold. How about dropping that fold, or moving the extended handling to before as below?

    https://github.com/Empact/bitcoin/commit/4b858645a2cdccd5d6c9a1610974676fcba2787a https://travis-ci.org/Empact/bitcoin/builds/409306396

14. scravy renamed this:
    travis : improve readability of travis.yml and log outputs
    travis: improve readability of travis.yml and log outputs
    on Jul 28, 2018
15. 
    scravy commented at 7:51 AM on July 30, 2018: contributor

    @Empact I removed the fold for the mere configuration of the extended tests.

16. 
    MarcoFalke commented at 4:32 PM on July 30, 2018: member

    Please squash the "fold" commits, see https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits

17. 
    Empact commented at 5:23 PM on July 30, 2018: member

    utACK 4639bbc, I checked that commands were faithfully refactored in 9730d3a; please squash latter 3 commits.

18. use travis:fold in test script steps f98146d001
19. scravy force-pushed on Jul 30, 2018
20. 
    scravy commented at 6:05 PM on July 30, 2018: contributor

    @Empact @MarcoFalke I squashed the fold-commits into one.

21. MarcoFalke merged this on Jul 30, 2018
22. MarcoFalke closed this on Jul 30, 2018

    ---

23. MarcoFalke referenced this in commit d25079ac71 on Jul 30, 2018
24. scravy deleted the branch on Aug 2, 2018
25. scravy cross-referenced this on Aug 6, 2018 from issue extract script from .travis.yml + generally improve readability by scravy
26. bitcoin locked this on Sep 8, 2021

Contributors

scravyMarcoFalkepracticalswiftDrahtBotken2812221EmpactFuzzbawls

Review Requested
MarcoFalke