Skip to:
Content

BuddyPress.org

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#8421 closed task (fixed)

Migrate from Travis to GitHub Actions

Reported by: slaffik's profile slaFFik Owned by: slaffik's profile slaFFik
Milestone: 8.0.0 Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch commit
Cc:

Description (last modified by slaFFik)

Travis is stopping its support of OSS, so we may consider migrating to GH Actions.
But first, we need to investigate its speed and overall performance.

Let's start with a GH workflow, that will run all the same tests/checks on the latest stable WordPress and PHP 7.4 (because that's the recommended PHP version by WordPress).

Example current Travis run: https://travis-ci.org/github/buddypress/BuddyPress/builds/750912512

Attachments (5)

8421.patch (3.7 KB) - added by imath 3 years ago.
8421-1.patch (346.3 KB) - added by espellcaste 3 years ago.
8421-2.patch (9.2 KB) - added by espellcaste 3 years ago.
8421.3.patch (9.2 KB) - added by imath 3 years ago.
8421.4.patch (5.5 KB) - added by imath 3 years ago.

Download all attachments as: .zip

Change History (24)

#1 @slaFFik
3 years ago

  • Component changed from Core to Build/Test Tools

#2 @slaFFik
3 years ago

  • Description modified (diff)

#3 @imath
3 years ago

  • Keywords has-patch added

Hi @slaFFik

Have you progressed on this task lately?

I've been exploring the subject myself and I was wondering what was your opinion about using the wp-env to run the PHP unit tests GitHub action?

Since #8381 has been fixed, I've tested such a GitHub action from a branch of my BuddyPress fork. It's testing BuddyPress against WordPress trunk and PHP 7.4, 8.0.

I had an issue with the slug we use on GitHub for BuddyPress (using buddypress wasn't working while BuddyPress works..)

FYI the attached patch is containing the action workflow adapted to the master branch.

Last edited 3 years ago by imath (previous) (diff)

@imath
3 years ago

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


3 years ago

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


3 years ago

#6 @slaFFik
3 years ago

Leaving here a link to my current state of this task on Github: https://github.com/slaFFik/BuddyPress/pull/1

It worked for me when run for a single combination of WP and PHP versions (example successful run - https://github.com/slaFFik/BuddyPress/runs/1678336683), but when in parallel the whole matrix - something went wrong. I didn't have the time to finish it :(

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


3 years ago

#8 @espellcaste
3 years ago

Folks: I have a working implementation of the Github Actions migration from Travis. This patch:

  • updates the @wordpress/wp-env package to the latest version, ^4.0.0, since the current one has some issues with docker.
  • introduces a PHPCS (coding standard) workflow action, which is not failing for any serious errors. This is related to #7228
  • introduces an unit tests workflow action. Currently only PHP 7.4 and 8.0 are tested. WordPress 5.7 and WordPress master.

The main thing I'd like someone to test locally is the phpunit-wp-config.php changes. The 4.0 version of the @wordpress/wp-env package updated the test mysql database credentials, https://github.com/WordPress/gutenberg/blob/trunk/packages/env/lib/config/db-env.js#L8-L11, forcing us to update it here.

Please, make sure your local version has the path as BuddyPress in the package.json script command.

Here is a temporary pull request I've been using for testing purposes: https://github.com/renatonascalves/BuddyPress/pull/1

The main questions I have:

  • what matrix should we be using on Github Actions? Testing everything might not be ideal for now because of the limits for Github Actions.
  • Should we test for all npm test commands? test-php:group and test-php-multisite:group as well?

Let me know what you think.

Last edited 3 years ago by espellcaste (previous) (diff)

@espellcaste
3 years ago

#9 @imath
3 years ago

Thanks a lot @espellcaste It looks very promising. I’ll test the patch asap. As we still have Travis for now, I believe the matrix you chose is a good start and enough. I’ll give you update asap 👌

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


3 years ago

#11 @imath
3 years ago

  • Keywords 2nd-opinion added

One quick think: @johnjamesjacoby what about changing the name of the BuddyPress GitHub repository to buddypress instead of BuddyPress. It would avoid this:

Please, make sure your local version has the path as BuddyPress in the package.json script command.

& a difference from our SVN repository.

I believe this caps change would have no effect on fetching files (GitHub would display a warning: please update origin/upstream to buddypress.

#12 @johnjamesjacoby
3 years ago

@imath renamed. Let’s see if anything else breaks! 💥

#13 @imath
3 years ago

  • Keywords needs-refresh added; has-patch removed

Thanks a lot @johnjamesjacoby 😍. So far so good. Here's the message you get when you use the old name:

Counting objects: 1, done.
Writing objects: 100% (1/1), 237 bytes | 237.00 KiB/s, done.
Total 1 (delta 0), reused 0 (delta 0)
remote: This repository moved. Please use the new location:
remote:   https://github.com/imath/BuddyPress.git
To https://github.com/imath/buddypress.git
   022ca0d51..112a9c035  master -> master

SVN sync is fine, as well as Travis tests.

I believe we can stay this way 👍

@espellcaste could you update the patch to stop using BuddyPress into your npm commands. Make sure to rename your fork the way @johnjamesjacoby just did > buddypress.

No emergency: I'll be able to review it once the 8.0.0 beta has been released (probably in ~11 hours from now)

@espellcaste
3 years ago

#14 @espellcaste
3 years ago

@imath Updated! I left both npm-shrinkwrap.json and composer.lock out. :)

@imath
3 years ago

#15 @imath
3 years ago

  • Keywords has-patch added; 2nd-opinion needs-refresh removed

Hi @espellcaste

Thanks a lot for your great work on the patch 💪. It was failing at first because of a wrong indentation around the on keyword. See https://github.com/imath/buddypress/actions/runs/804600937

So 8421.3.patch is fixing it along with updating the patch with latest BP Trunk. (Parcel was already updated to 1.12.5 in package.json and wp-phpunit was > 5.7 in composer.json).

And it's working!

https://cldup.com/_lPXBuwzJT.png

So to apply the patch, here's my suggestion:

  • remove the composer.lock file and /vendor directory, then run composer install
  • remove the npm-shrinwrap.json file and then run npm install.

I believe the PHPCS part needs to be more meaningful eg: testing buddypress text domains or/and `/* translators: */ inline comments in translation or/and testing PHP Compatibility for example. Otherwise let's just remove these "Coding Standards" jobs.

I'm fine with committing the unit tests part the way it is 👍

@imath
3 years ago

#16 @imath
3 years ago

  • Keywords commit added

As discussed during this dev-chat, 8421.4.patch only contains the PHP Unit tests part. I've just tested it again and got 4 green lights. I will commit this patch asap.

#17 @imath
3 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 12950:

Build/test tools: use GitHub Actions to run PHP unit tests

We're starting using GitHub Actions with these 4 jobs to do our PHP Unit tests on this matrix :

  • WP Trunk - PHP 7.4
  • WP Trunk - PHP 8.0
  • WP 5.7 - PHP 7.4
  • WP 5.7 - PHP 8.0

As the future of Travis.org is uncertain, we believe it's an important step to secure our testing tool.

Props slaFFik, Huge Props espellcaste 💪

Fixes #8421

#18 @imath
3 years ago

In 12952:

Sync Trunk's readme.txt with the WP plugins directory one

This commit also improves our GH repo README.md file to include a badge showing latest results of our newly introduced GH Action.

See #8421

#19 @slaFFik
3 years ago

@imath I would suggest adding support for [skip ci] to skip GH Actions when readme/changelog/images or other files that do not modify the code are pushed, and there is no reason to run all the tests.

Example:

runs-on: ubuntu-20.04
if: "!contains(github.event.head_commit.message, '[skip ci]')"

I would also recommend sticking to an exact runs-on: ubuntu-20.04 instead of relying on a more generic ubuntu-latest (which you will need to monitor from time to time).

Note: See TracTickets for help on using tickets.