Skip to content
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

This PR implements a number of improvements for our Travis CI: #361

Closed
wants to merge 10 commits into from

Conversation

martbab
Copy link
Contributor

@martbab martbab commented Dec 21, 2016

  • split out the test runner part into a standalone script, .travis.yml should
    now only define test matrix, set environment variables and process output
    after failure

  • mark the project as Python one, implement support for running builds using
    different python version (future-proofing against incoming Python2/3 CI) and
    cache job dependencies

  • use separate job for pep8/linters. This shaves off ca 6-8 minutes from
    overall build time. You should get CI results in 26 min compared to previous
    33 min

Martin Babinsky added 8 commits December 21, 2016 15:16
This prevents Travis log collector to add separate expansion marks to
the echo output and the actuall log output.
this script is intended only for use in Travis CI and contains
configuration of the test run requested:

    * it can run linter step separately by specifying TASK_TO_RUN="lint"
      environment variable in .travis.yml. In this case it also runs
      pep8 checker on the commits in PR.
    * other steps are run in developer mode in order to skip pylint run
      and speed up the task
    * in all cases the CI result log is populated and can be displayed
      if the job fails
In order to speed our Travis CI gating even further, the lint step has
been split to a separate job that can be run in parallel with the test
runs. The test runs are in turn launched in developer mode to speed them
up.
This is a preparatory work for the future requirement of running
Python2/3 jobs simultaneously.
@martbab
Copy link
Contributor Author

martbab commented Jan 2, 2017

Bump for review.

Copy link
Contributor

@frasertweedale frasertweedale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Travis shows that the desired change was effected :)

@stlaz
Copy link
Contributor

stlaz commented Jan 4, 2017

I assume the licence headers did not break the automember tests so this could be pushed.

Just a brief question: would it be reasonable to get the line number of "========= FAILURES =========" and tail the "$CI_TRAVIS_LOG" from the end to it?

@martbab
Copy link
Contributor Author

martbab commented Jan 4, 2017

@stlaz thanks for an idea, that will actually make the log output much more useful. I will try to think about it.

@stlaz
Copy link
Contributor

stlaz commented Jan 4, 2017

@martbab My naive solution is to do something like

LINE=`grep -n -m 1 $CI_TRAVIS_LOG -e "=== FAILURES ===" | cut -d: -f1`
LINES=`wc -l $CI_TRAVIS_LOG`
tail -n `expr $LINES - $LINE` $CI_TRAVIS_LOG

@martbab
Copy link
Contributor Author

martbab commented Jan 5, 2017

@stlaz I have implemented a simple log trimming which keeps only pytest failures if present. The original behavior is kept as a fallback for the case if the setup fails.

@stlaz
Copy link
Contributor

stlaz commented Jan 5, 2017

The change LGTM, ACK, we'll see how it works :)

@stlaz stlaz added ack Pull Request approved, can be merged and removed ack Pull Request approved, can be merged labels Jan 5, 2017
function truncate_log_to_test_failures() {
# chop off everything in the CI_RESULTS_LOG preceding pytest error output
# if there are pytest errors in the log
$error_fail_regexp='\(=== ERRORS ===\)\|\(=== FAILURES ===\)'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should $error_fail_regexp miss "$"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops sorry I forgot how to bash.

# if there are pytest errors in the log
$error_fail_regexp='\(=== ERRORS ===\)\|\(=== FAILURES ===\)'

if grep -e "$error_fail_regexp" $CI_RESULTS_LOG
Copy link
Contributor

@stlaz stlaz Jan 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add > /dev/null

If we get to the `run-tests` phase, we no longer care about messages
from previous steps so we can truncate the log output to only show
pytest failures/errors, reducing log size.

As a fallback, the previous behaviour to print last 5000 log lines was
kept in the case the CI tests fail during setup.
@stlaz
Copy link
Contributor

stlaz commented Jan 5, 2017

I have no more remarks on this, hopefully final ACK.

@stlaz stlaz added the ack Pull Request approved, can be merged label Jan 5, 2017
@martbab martbab added the pushed Pull Request has already been pushed label Jan 5, 2017
@martbab martbab closed this Jan 5, 2017
@martbab martbab deleted the travis-ci-improvements branch January 5, 2017 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
3 participants