Make WordPress Core

Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#28301 closed enhancement (fixed)

Improved Travis CI Performance and Configuration

Reported by: netweb's profile netweb Owned by: wonderboymusic's profile wonderboymusic
Milestone: 4.0 Priority: normal
Severity: normal Version: 4.0
Component: Build/Test Tools Keywords: has-patch
Focuses: performance Cc:

Description

Based on this comment by Jorbin: ticket:26446#comment:2

I would love to figure out a way to not have to run jshint for each of the configurations since you are correct that it doesn't need to be run across multiple build environments. I'm not sure if Travis supports that, but I imagine we can code around that if you really think it is a show stopper. Of course, as you point out we are only adding ~16 seconds to the travis build.

The attached patch does the above and a few more things:

  • Switches to a using a 'build matrix' with explicitly defined PHP version and environment variable for each build job, each environment variable is a new Grunt task.
  • Existing build jobs no longer include the qunit:compiled or jshint tests, they each have their own dedicated build job using PHP 5.5 and do NOT include PHPUnit testing, MySQL database creation, or SVN checkout of WordPress Importer. (No specific reason I chose PHP 5.5, I had to pick one is all)
  • The job build order is prioritized so that the qunit and javascript tests are run first, both of these tasks have in testing remained under 120 seconds and 90 seconds respectively.
  • The addition of the fast_finish: true flag in the build matrix will mark the build passed or errored as soon as a single build job has failed. Previously the build status was not reported until ALL tests had completed regardless of status. In theory if a jshint test failed previously you would not know for ~30 minutes, this should now report in under two minutes once the build job has begun.
  • Moves the 'build configuration' to before_install which includes only if the build job is travis:phpunit the MySQL database creation, and WordPress Importer SVN checkout. When a failure occurs in this section the build is marked as errored which is a more accurate description in that the environment we are setting up has failed and not the actual WordPress tests we are testing.
  • Introduces and/or modifies existing Grunt test tasks for Travis CI testing, this allows each task to be targeted via the new environment variables in the build matrix called now by script: grunt $WP_TRAVISCI rather than the existing grunt travis command.
    • grunt.registerTask('travis:js', 'Runs Javascript TravisCI tasks.', 'jshint');
    • grunt.registerTask('travis:phpunit', 'Runs PHPUnit TravisCI tasks.', 'phpunit');
    • grunt.registerTask('travis:qunit', 'Runs QUnit TravisCI tasks.', 'qunit:compiled');
  • Introduces PHP 5.6 and HHVM testing into the build matrix, both of these are allowed to fail in that they will not be included in the overall build job failed, errored, or passed build status. This allows WordPress testing to play nice with these alpha/beta version of PHP/HHVM until stable and officially released.

Build Matrix Summary

DurationENVPHPAllowed Failures
1 min 45 secWP_TRAVISCI=travis:qunit5.5No
1 min 10 secWP_TRAVISCI=travis:js5.5No
5 min 2 secWP_TRAVISCI=travis:phpunit5.2No
7 min 46 secWP_TRAVISCI=travis:phpunit5.3No
8 min 13 secWP_TRAVISCI=travis:phpunit5.4No
9 min 5 secWP_TRAVISCI=travis:phpunit5.5No
7 min 43 secWP_TRAVISCI=travis:phpunit5.6Yes
1 min 22 secWP_TRAVISCI=travis:phpunithhvmYes

A couple of the Travis CI tests from my fork for this patch:
https://travis-ci.org/ntwb/wordpress/builds/25441345 <- A forced errored build
https://travis-ci.org/ntwb/wordpress/builds/25495226 <- The build this patch was submitted against

Attachments (2)

28301.patch (3.0 KB) - added by netweb 10 years ago.
28301.diff (2.9 KB) - added by jorbin 10 years ago.

Download all attachments as: .zip

Change History (9)

@netweb
10 years ago

#1 @jorbin
10 years ago

  • Milestone changed from Awaiting Review to 4.0

For the most part, I like this.

I don't think that we need to separate out jshint and qunit into separate tasks for travis. They can be run at the same time in my opinion.

I also don't think we should remove the test task. That comes in handy if you want to run both phpunit and qunit.

I really like the addition of running our tests (though with failures allowed) against both hhvm and php5.6

#2 @netweb
10 years ago

Hehehe... Jorbin, thanks for the feedback and the following comment of yours in ticket:26446#comment:2 was how the separate tasks came about ;)

I would love to figure out a way to not have to run jshint for each of the configurations since you are correct that it doesn't need to be run across multiple build environments. I'm not sure if Travis supports that, but I imagine we can code around that if you really think it is a show stopper. Of course, as you point out we are only adding ~16 seconds to the travis build.

I'm not particularly concerned which way we go in laying out the test matrix, these can always be revisited down the road when all the PHPUnit multisite tests start passing.

I will put the test task back in and then a couple of options based on more feedback above:

  1. For each PHP version do not include the jshint/qunit tests and use separate jshint/qunit test (original patch)
  2. For each PHP version include the jshint/qunit tests and the standalone jshint/qunit tests
  3. For each PHP version include the jshint/qunit tests without standalone jshint/qunit tests
  4. Some combination I haven't thought of

My first preference is 1, followed by 2 as the jshint and qunit tests do currently (and should) always pass testing and feedback of a failure can be determined in under ~2 mins (theoretically if the tests start shortly after commit)

@jorbin
10 years ago

#3 @jorbin
10 years ago

  • Keywords has-patch added

Sorry, I think I worded my comment bad.

I think we should keep the grunt test task, but not use it for travis. And we should combine the travis:js and travis:qunit tasks. I've uploaded that change.

#4 @netweb
10 years ago

Cool, gotcha, done, thanks, with your 28301.diff patch -> travis-ci.org/ntwb/wordpress Build #37

This ticket was mentioned in IRC in #wordpress-dev by jorbin. View the logs.


10 years ago

#6 @wonderboymusic
10 years ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

In 28799:

Improve Travis CI configuration for better performance.

Props netweb, jorbin.
Fixes #28301.

This ticket was mentioned in Slack in #core by netweb. View the logs.


9 years ago

Note: See TracTickets for help on using tickets.