[tests] Remove travis_wait from lint script #15255

1. 
   gkrizek commented at 4:30 AM on January 25, 2019: contributor

   Using the travis_wait command in conjunction with set -o errexit causes problems. The travis_wait command will correctly log the command's output if successful, but if the command fails the process exits before the travis_wait command can dump the logs. This will hide important debugging information like error messages and stack traces. We ran into this in #15196 and it was very hard to debug because output was being suppressed.

   travis_wait was being used because the contrib/verify-commits/verify-commits.py script can sometimes run for a long time without producing any output. If a script runs for 10 minutes without logging anything, the CI run times out. The travis_wait command will extend this timeout by logging a message for you, while sending stderr and stdout to a file.

   This PR removes the travis_wait command from our CI system and adds additional logging to the verify-commits.py script so it doesn't make Travis timeout.

2. in contrib/verify-commits/verify-commits.py:158 in 6d248b0620 outdated

    154 | @@ -150,6 +155,7 @@ def main():
    155 |  
    156 |          prev_commit = current_commit
    157 |          current_commit = parents[0]
    158 | +        commit_number += 0
   

   ---

   ---

   

   Empact commented at 4:39 AM on January 25, 2019:

   This will never log since it doesn't increment

   ---

   

   gkrizek commented at 4:43 AM on January 25, 2019:

   Wow! Stupid mistake, haha. Thanks for pointing that out.

3. 
   Empact commented at 4:39 AM on January 25, 2019: member

   Concept ACK I've seen this as well

4. gkrizek force-pushed on Jan 25, 2019
5. 
   Empact commented at 4:53 AM on January 25, 2019: member

   ~Possibly fixed in travis-build master? https://github.com/travis-ci/travis-build/pull/1162 https://gist.github.com/letmaik/caa0f6cc4375cbfcc1ff26bd4530c2a3~

6. 
   gkrizek commented at 5:05 AM on January 25, 2019: contributor

   @Empact Looking at that PR, it seems like that logic was only added to travis_retry and not travis_wait. When this first happened I posted on their forums to get some clarification and their staff pointed out the errexit conflict.

   Also, looking at the travis_wait command on Travis master, seems to not catch errors https://github.com/travis-ci/travis-build/blob/4f580b238530108cdd08719c326cd571d4e7b99f/lib/travis/build/bash/travis_wait.bash#L13

7. fanquake added the label Tests on Jan 25, 2019
8. fanquake added the label Scripts and tools on Jan 25, 2019
9. laanwj requested review from theuni on Jan 30, 2019
10. in contrib/verify-commits/verify-commits.py:101 in e8d2b2d9e4 outdated

      93 | @@ -94,7 +94,12 @@ def main():
      94 |      branch = subprocess.check_output([GIT, 'show', '-s', '--format=%H', initial_commit]).decode('utf8').splitlines()[0]
      95 |  
      96 |      # Iterate through commits
      97 | +    commit_number = 1
      98 |      while True:
      99 | +        # Log a message to prevent Travis from timing out
     100 | +        if commit_number % 50 == 0:
     101 | +            print("verify-commits: [in-progress] processing commit number {} ({})".format(commit_number, current_commit[:8]))
    

    ---

    ---

    

    MarcoFalke commented at 8:20 PM on January 30, 2019:

    If you print anyway, why not print every commit, but make it a log that is only enabled with log level debug?

    ---

    

    gkrizek commented at 8:29 PM on January 30, 2019:

    We certainly can do that. I didn't because I thought every commit would be too much output and make the Travis logs messy.

    I have no problem logging every commit if we think that's the preferred solution.

11. 
    MarcoFalke commented at 8:21 PM on January 30, 2019: member

    Concept ACK

12. Remove travis_wait from lint script

    Also adding progress logging to verify-commits.py script to prevent Travis from timing out

    8b8d8eeae9
13. gkrizek force-pushed on Feb 21, 2019
14. 
    gkrizek commented at 5:14 PM on February 21, 2019: contributor

    This is updated with @MarcoFalke 's suggestion. This will now detect if the script is running in CI and if it is log level is set to debug. Then it will log every commit that's inspected to prevent Travis from timing out.

15. 
    DrahtBot commented at 12:07 AM on March 29, 2019: member

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #15693 (travis: Switch to ubuntu keyserver to avoid timeouts by MarcoFalke)

    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.

16. 
    MarcoFalke commented at 1:42 PM on March 29, 2019: member

    utACK 8b8d8eeae9e8feff6d78420ee172c820ccef9db1

17. MarcoFalke referenced this in commit 904129b35d on Mar 29, 2019
18. MarcoFalke merged this on Mar 29, 2019
19. MarcoFalke closed this on Mar 29, 2019

    ---

20. PastaPastaPasta referenced this in commit 96426b5057 on Sep 11, 2021
21. DrahtBot locked this on Dec 16, 2021

Contributors

gkrizekEmpactMarcoFalkeDrahtBot

Review Requested
theuni