Skip to:
Content

BuddyPress.org

Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#6726 closed defect (bug) (fixed)

Run Grunt build tasks in Travis

Reported by: netweb Owned by: boonebgorges
Milestone: 2.6 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords:
Cc:

Description

Per BP dev chat November 11th https://wordpress.slack.com/archives/buddypress/p1447266682000057

boone [5:31 AM] We should run build tasks in Travis so that we catch this kind of stuff before we go to release

Attachments (3)

6726.diff (505 bytes) - added by netweb 5 years ago.
6726.1.diff (2.2 KB) - added by netweb 5 years ago.
6726.2.patch (1.9 KB) - added by netweb 4 years ago.

Download all attachments as: .zip

Change History (20)

@netweb
5 years ago

#1 @netweb
5 years ago

This 6726.diff patch takes the approach of running grunt build before grunt test as part of the grunt travis tasks for each and every Travis job.

The alternative approach is to create a single standalone Travis job that does not run PHPunit, only Grunt tasks similar to WordPress Core approach, e.g https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/90628321

#2 @DJPaul
5 years ago

  • Milestone changed from Awaiting Review to Under Consideration

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


5 years ago

#4 @DJPaul
5 years ago

Running builds in travis-ci will make each job take much, much longer to complete. Not keen. :(

#5 @boonebgorges
5 years ago

It's unfortunate that our current Travis builds take so long. But exhaustive (and boring) tests are the very reason for having asynchronous, external test builds.

If we don't like the idea of running Grunt tasks after every commit, we might consider setting up another Travis build that runs only once per day, with a more extensive test matrix. (We could even move some of our current tests there - like those run against older versions of WP.)

#6 @netweb
5 years ago

We could add a new Travis job as part of the build matrix that runs the full suite of Grunt tasks, similar to how the WordPress "JS Only" task runs but for Grunt, that would allow the Grunt tasks to run but not affect existing jobs.

If you come with a "live" and "daily" build matrix I'll make the patch/patches to make it happen.

  • Maybe something along the lines of:
    • WP 3.8 PHP 5.2
    • WP 3.9 PHP 5.3
    • WP 4.0 PHP 5.4
    • WP 4.1 PHP 5.5
    • WP 4.3 PHP 5.6
    • WP 4.4 PHP 7
    • WP Master/Trunk PHP 5.2, 5.3, 5.4, 5.5, 5.6, 7
  • That would make 12 jobs "live" and offload 26 jobs to a once "daily" matrix

#7 follow-up: @DJPaul
5 years ago

If the purpose of testing grunt build is to catch any upstream errors or problems with the dependencies, maybe we only need to run it once? Not for every possible matrix combination.

#8 in reply to: ↑ 7 @netweb
5 years ago

Replying to DJPaul:

...maybe we only need to run it once? Not for every possible matrix combination.

Yes, thats what I was thinking, will whip up a patch to do that.

#9 @DJPaul
5 years ago

  • Milestone changed from Under Consideration to 2.6

Let's discuss this again for 2.6.

@netweb
5 years ago

#10 @netweb
5 years ago

In 6726.1.diff a single Travis job is added that only runs grunt build and does not run any PHPUnit tests.

Example Travis CI Build and Job:

#11 follow-up: @DJPaul
5 years ago

@boonebgorges what do you think? would this be enough to catch whatever the problem we had before for 2.4 was? I can't really pick up from the Slack notes what the problem was, I know you and @imath were looking at it.

#12 in reply to: ↑ 11 @netweb
5 years ago

Replying to DJPaul:

@boonebgorges what do you think? would this be enough to catch whatever the problem we had before for 2.4 was? I can't really pick up from the Slack notes what the problem was, I know you and @imath were looking at it.

The previous page of the Slack notes is probably more helpful, the crux of it was:

imath [5:07 AM] i have a problem with imagemin-gifsicle

ok first is a fail. Here's the error i get when trying to grunt build : Warning: Error: spawn /Users/imath/build-bp/buddypress/node_modules/grunt-contrib-imagemin/node_modules/imagemin/node_modules/imagemin-gifsicle/node_modules/gifsicle/vendor/gifsicle ENOENT in file src/bp-core/admin/images/accessibility.gif Use --force to continue.

An NPM module was not playing well, hence running a Travis CI job for grunt build "should" pick up issues like this.

#13 follow-up: @boonebgorges
4 years ago

  • Keywords needs-refresh added; has-patch removed

@netweb Sorry to be dense - can you explain what 6726.1.diff does? Is this meant to be a patch for our trunk .travis.yml? It looks like it runs a single grunt build job to every build. Is that right? Or is this meant to describe the once-daily matrix? I'm getting a bit bleary-eyed at the double- and triple-negatives in the matrix definition :)

@netweb
4 years ago

#14 in reply to: ↑ 13 @netweb
4 years ago

Replying to boonebgorges:

@netweb Sorry to be dense - can you explain what 6726.1.diff does?

Sure

Is this meant to be a patch for our trunk .travis.yml?

Yes

It looks like it runs a single grunt build job to every build. Is that right?

Incorrect, the 6726.1.diff patch adds 1 extra job to the build matrix to run a PHP 7 job that runs grunt build once

It is not part of any of the existing jobs, the existing jobs remain unchanged and will continue to perform exactly as they currently do.

Or is this meant to describe the once-daily matrix?

No

I'm getting a bit bleary-eyed at the double- and triple-negatives in the matrix definition :)

The patch 6726.2.patch is only a refresh of 6726.1.diff but probably still doesn't make a whole lot of sense, the two key pieces are adding Travis environment variables BP_TRAVISCI=travis:grunt and BP_TRAVISCI=travis:phpunit to either the Grunt or PHPUnit Travis CI jobs, each job is then passed these environment variables and ran via script: grunt $BP_TRAVISCI for each type of job

Currently there are 34 jobs per Travis CI build plus an additional job for PHP7 Nightly totalling 35 jobs per build

Using 6726.2.patch an extra job 50.36 is added to the build matrix taking the total to 36 jobs per build.

The primary purpose of adding this job per the original Slack chat was to provide a way to pickup utilising Travis CI for any potential failures running the primary Grunt tasks. Previously the existing tasks only ran grunt travis:phpunit which in turn runs each of these grunt tasks: grunt jsvalidate:src, grunt jshint, grunt checktextdomain, grunt test, the "main" Grunt task grunt build was previously never ran within Travis CI.

Last edited 4 years ago by netweb (previous) (diff)

#15 @boonebgorges
4 years ago

  • Keywords needs-refresh removed

Thanks, @netweb!

It looks like it runs a single grunt build job to every build. Is that right?

Incorrect, the 6726.1.diff​ patch adds 1 extra job to the build matrix to run a PHP 7 job that runs grunt build once

Er, it sounds like you meant "correct".

This looks good with me. Let's do it.

#16 @boonebgorges
4 years ago

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

In 10771:

Add a grunt build job to the Travis test matrix.

Running the build process in CI should help to catch errors (botched
dependencies and the like) before we actually go to package a release.

Props netweb.
Fixes #6726.

#17 @DJPaul
4 years ago

  • Component changed from Tools - Build Process to Build/Test Tools
Note: See TracTickets for help on using tickets.