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.

Sign up for GitHub

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

Jump to bottom

Fetch WC L-1 versions #2046

Merged
merged 11 commits into from
Aug 9, 2023
Merged

Fetch WC L-1 versions #2046

Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1f6dee5
Fetch WC L-1 versions
puntope Aug 7, 2023
daca6c6
Change line for trigger Workflow
puntope Aug 7, 2023
e03e875
Fix Workflow
puntope Aug 7, 2023
cba798e
Fix Workflow
puntope Aug 7, 2023
991fa30
Fix Workflow
puntope Aug 7, 2023
761a697
Fix Workflow
puntope Aug 7, 2023
16083fc
Fix Workflow
puntope Aug 7, 2023
3ffc080
Fix Workflow
puntope Aug 7, 2023
9e3334f
Fix Workflow
puntope Aug 7, 2023
f384876
Refactor Workflow matrix
puntope Aug 8, 2023
2da9e70
Fix typos
puntope Aug 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 28 additions & 10 deletions .github/workflows/php-unit-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ concurrency:
cancel-in-progress: true

jobs:
GetMatrix:
name: Get WP version Matrix
GetWPMatrix:
name: Get WP L-2 version Matrix
runs-on: ubuntu-latest
outputs:
wp-versions: ${{ steps.wp.outputs.versions }}
Expand All @@ -35,24 +35,42 @@ jobs:
with:
slug: wordpress

GetWCMatrix:
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as comment https://github.com/woocommerce/automatewoo/pull/1527/files#r1286879075
Does it make sense to merge this as one job?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done ✅

name: Get WC L-1 version Matrix
runs-on: ubuntu-latest
outputs:
previous-wc-version: ${{ fromJson(steps.wc.outputs.versions)[1] }}
latest-wc-version: ${{ fromJson(steps.wc.outputs.versions)[0] }}
steps:
- name: Get Release versions from WooCommerce
id: wc
uses: woocommerce/grow/get-plugin-releases@actions-v1
with:
slug: woocommerce
releases: 2

UnitTests:
name: PHP unit tests - PHP ${{ matrix.php }}, WP ${{ matrix.wp-version }}
needs: GetMatrix
name: PHP unit tests - PHP ${{ matrix.php }}, WP ${{ matrix.wp-version }}, WC ${{ matrix.wc-version || 'latest' }}
needs: [GetWCMatrix, GetWPMatrix]
runs-on: ubuntu-latest
env:
WP_CORE_DIR: "/tmp/wordpress/src"
WP_TESTS_DIR: "/tmp/wordpress/tests/phpunit"
strategy:
matrix:
php: [8.0]
wp-version: ${{ fromJson(needs.GetMatrix.outputs.wp-versions) }}
wp-version: ${{ fromJson(needs.GetWPMatrix.outputs.wp-versions) }}
wc-version: [ latest ]
include:
- php: 7.4
wp-version: ${{ needs.GetMatrix.outputs.latest-wp-version }}
wp-version: ${{ needs.GetWPMatrix.outputs.latest-wp-version }}
wc-version: ${{ needs.GetWCMatrix.outputs.previous-wc-version }}
Copy link
Contributor

Choose a reason for hiding this comment

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

In the interest of not testing all combinations (to save billing minutes), on other extensions we make this test do the oldest supported version of everything. So in that case wouldn't this need to be PHP 7.4, WP L-2, WC L-1.
Since this is the only test that uses an older version of WC I wouldn't be against changing this to WC L-2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the matrix, now we follow the same as Automatewoo

- php: 8.1
wp-version: ${{ needs.GetMatrix.outputs.latest-wp-version }}
wp-version: ${{ needs.GetWPMatrix.outputs.latest-wp-version }}
wc-version: ${{ needs.GetWCMatrix.outputs.latest-wc-version }}
- php: 8.2
wp-version: ${{ needs.GetMatrix.outputs.latest-wp-version }}
wp-version: ${{ needs.GetWPMatrix.outputs.latest-wp-version }}
wc-version: ${{ needs.GetWCMatrix.outputs.latest-wc-version }}

steps:
- name: Checkout repository
Expand All @@ -67,9 +85,9 @@ jobs:
uses: woocommerce/grow/prepare-mysql@actions-v1

- name: Install WP tests
run: ./bin/install-wp-tests.sh wordpress_test root root localhost ${{ matrix.wp-version }}
run: ./bin/install-wp-tests.sh wordpress_test root root localhost ${{ matrix.wp-version }} ${{ matrix.wc-version }}

- if: matrix.wp-version == needs.GetMatrix.outputs.latest-wp-version && matrix.php == 8.0
- if: matrix.wp-version == needs.GetWPMatrix.outputs.latest-wp-version && matrix.php == 8.0
Copy link
Contributor

Choose a reason for hiding this comment

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

We might as well add an additional check to make sure WC is at latest version to ensure the coverage report is only generated on one run. Right now it will still run only once but if we tweak the matrix later on then it would be easy to miss:

Suggested change
- if: matrix.wp-version == needs.GetWPMatrix.outputs.latest-wp-version && matrix.php == 8.0
- if: matrix.wp-version == needs.GetWPMatrix.outputs.latest-wp-version && matrix.wc-version == needs.GetWCMatrix.outputs.latest-wc-version && matrix.php == 8.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is done as well, as we are now changing the matrix I check PHP 8.0 and WC latest

Copy link
Contributor

Choose a reason for hiding this comment

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

The suggestion to check all three was in case we would test more combinations of WP as well. But not a big deal since as you mentioned it is cover how it is now.

name: Set condition to generate coverage report (only on latest versions)
run: echo "generate_coverage=true" >> $GITHUB_ENV

Expand Down
Loading