New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TravisCI: More robust parsing of TravisCI log #395

Merged
merged 10 commits into from Jun 20, 2016

Conversation

Projects
None yet
3 participants
@tammoippen
Contributor

tammoippen commented Jun 13, 2016

Solves #392.
This PR makes the parsing of the TravisCI more robust and allows to order the different steps of the build.sh in any order (for the parsing). It is also possible to remove different steps completely and the parser will not break. A summary of the changes:

In build.sh:

  • Every block is surrounded by tags like

    ======= Extract changed files start =======
    ...
    ======= Extract changed files end =======
    
  • Output is reduced:

    • Comment set -x out, as we do not need to debug the travis log regularly. One can uncomment it, if needed.
    • Remove verbose flag of extracting clang-format.
  • Make static analysis optional with env. variable xSTATIC_ANALYSIS.

  • Fix regex bug (see line 4188) for if [[ $f =~ $EXAMPLE_DIRS ]]. I think, the TravisCI environment does not allow if [[ ... ]].

  • Add output that tells if results are not uploaded, if you are working on your fork.

In extras/parse_travis_log.py:

  • No state on reading/parsing the log. Every function opens the file itself and completely parses it for its issues. The actual parsing happens only within the starting/ending tags relevant for this function.
  • Separate vera++, cppcheck and clang-format parsing into standalone functions.
  • Add print_... functions for nicer output.
  • Cleanup "main" function.

I propose @heplesser and @gtrensch as reviewers.

@gtrensch

This comment has been minimized.

Show comment
Hide comment
@gtrensch

gtrensch Jun 17, 2016

Contributor

👍 Everything works fine! Just one remark from my side: There is a comment in parse_travis_log.py referring to the dependencies on build.sh. A similar comment in build.sh would be good.

Contributor

gtrensch commented Jun 17, 2016

👍 Everything works fine! Just one remark from my side: There is a comment in parse_travis_log.py referring to the dependencies on build.sh. A similar comment in build.sh would be good.

Show outdated Hide outdated build.sh
@@ -1,7 +1,7 @@
#!/bin/sh
set -e
set -x
#set -x

This comment has been minimized.

@heplesser

heplesser Jun 18, 2016

Contributor

Why commented out instead of deleted?

@heplesser

heplesser Jun 18, 2016

Contributor

Why commented out instead of deleted?

This comment has been minimized.

@tammoippen

tammoippen Jun 20, 2016

Contributor

@heplesser I documented the 'out-commenting'. When debugging the script, the flag is very informative.

@tammoippen

tammoippen Jun 20, 2016

Contributor

@heplesser I documented the 'out-commenting'. When debugging the script, the flag is very informative.

Show outdated Hide outdated build.sh
echo "There are files with a formatting error: $format_error_files ."
exit 42
fi
fi

This comment has been minimized.

@heplesser

heplesser Jun 19, 2016

Contributor

Please add a comment showing that this fi matches if STATIC_ANALYSIS about 150 lines further up; for readabilities sake.

@heplesser

heplesser Jun 19, 2016

Contributor

Please add a comment showing that this fi matches if STATIC_ANALYSIS about 150 lines further up; for readabilities sake.

Show outdated Hide outdated extras/parse_travis_log.py
if (not in_changed_files_section and
line.strip() == "======= Extract changed files start ======="):
in_changed_files_section = True
if in_changed_files_section and line.startswith('file_names='):

This comment has been minimized.

@heplesser

heplesser Jun 19, 2016

Contributor

Shouldn't this be line.strip().startswith('file_names=') to be on the safe side? Applies also to all other uses of line.startswith().

@heplesser

heplesser Jun 19, 2016

Contributor

Shouldn't this be line.strip().startswith('file_names=') to be on the safe side? Applies also to all other uses of line.startswith().

This comment has been minimized.

@tammoippen

tammoippen Jun 20, 2016

Contributor

@heplesser I added the strip()'s in f1d5b15.

@tammoippen

tammoippen Jun 20, 2016

Contributor

@heplesser I added the strip()'s in f1d5b15.

@heplesser

This comment has been minimized.

Show comment
Hide comment
@heplesser

heplesser Jun 19, 2016

Contributor

@tammoippen Very nice work. Have you tested that errors are handled properly, by introducing changes that will provoke errors/warning from vera, clang-format, cppcheck, pep8, building, testsuite? I think that we need some intentionally provoked Travis failures to be sure things work as intended.

Contributor

heplesser commented Jun 19, 2016

@tammoippen Very nice work. Have you tested that errors are handled properly, by introducing changes that will provoke errors/warning from vera, clang-format, cppcheck, pep8, building, testsuite? I think that we need some intentionally provoked Travis failures to be sure things work as intended.

@tammoippen

This comment has been minimized.

Show comment
Hide comment
@tammoippen

tammoippen Jun 20, 2016

Contributor

@gtrensch I added the comment in 2e10e14. Thanks.

Contributor

tammoippen commented Jun 20, 2016

@gtrensch I added the comment in 2e10e14. Thanks.

@tammoippen

This comment has been minimized.

Show comment
Hide comment
@tammoippen

tammoippen Jun 20, 2016

Contributor

@heplesser in 30d525b I introduced some intentional failures that should trigger 'vera, clang-format, cppcheck, pep8, building, testsuite'. I will remove it afterwards.

Contributor

tammoippen commented Jun 20, 2016

@heplesser in 30d525b I introduced some intentional failures that should trigger 'vera, clang-format, cppcheck, pep8, building, testsuite'. I will remove it afterwards.

tammoippen added some commits Jun 10, 2016

Add status flags to build.sh
And make less verbose build.sh.
Allow skipping of parts in the log
Pep8ify parse_travis_log
Fix skip error return value
Comment dependency with parse_travis_log.py in build.sh
And add comment to `fi` of `if [ "$xSTATIC_ANALYSIS" = "1" ] ; then`
Improve robustness of parsing.
* strip() startswith queries
* standardize PEP8 start output in build.sh to other static analysis
* unique parsing of start-outputs
* make static analysis run for all jobs
* do not exit build.sh when there are files with changes
@tammoippen

This comment has been minimized.

Show comment
Hide comment
@tammoippen

tammoippen Jun 20, 2016

Contributor

See this build for forced errors.

Contributor

tammoippen commented Jun 20, 2016

See this build for forced errors.

@heplesser

This comment has been minimized.

Show comment
Hide comment
@heplesser

heplesser Jun 20, 2016

Contributor

👍 and merging.

Contributor

heplesser commented Jun 20, 2016

👍 and merging.

@heplesser heplesser merged commit 830084d into nest:master Jun 20, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@tammoippen tammoippen deleted the tammoippen:travisci branch Jul 1, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment