Skip to:
Content

BuddyPress.org

Opened 7 months ago

Closed 2 months ago

Last modified 2 months ago

#8421 closed task (fixed)

Migrate from Travis to GitHub Actions

Reported by: slaFFik Owned by: 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 5 months ago.
8421-1.patch (346.3 KB) - added by espellcaste 4 months ago.
8421-2.patch (9.2 KB) - added by espellcaste 3 months ago.
8421.3.patch (9.2 KB) - added by imath 3 months ago.
8421.4.patch (5.5 KB) - added by imath 2 months ago.

Download all attachments as: .zip

Change History (24)

#1 @slaFFik
7 months ago

  • Component changed from Core to Build/Test Tools

#2 @slaFFik
7 months ago

  • Description modified (diff)

#3 @imath
5 months 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 5 months ago by imath (previous) (diff)

@imath
5 months ago

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


4 months ago

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


4 months ago

#6 @slaFFik
4 months 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.


4 months ago

#8 @espellcaste
4 months 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 4 months ago by espellcaste (previous) (diff)

@espellcaste
4 months ago

#9 @imath
4 months 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 months ago

#11 @imath
3 months 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 months ago

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

#13 @imath
3 months 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 months ago

#14 @espellcaste
3 months ago

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

@imath
3 months ago

#15 @imath
3 months 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
2 months ago

#16 @imath
2 months 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
2 months 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
2 months 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
2 months 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.