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.

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

Add E2E support #240

Open
wants to merge 1 commit into
base: development
Choose a base branch
from
Open

Add E2E support #240

wants to merge 1 commit into from

Conversation

Soare-Robert-Daniel
Copy link
Contributor

@Soare-Robert-Daniel Soare-Robert-Daniel commented Jun 25, 2024

Add E2E test support via Playwright and Gutenberg utils packages.

Implemented test:

  • check rendering on the About Us page
  • Script Loader module
  • Rollback module
  • Promotions module
  • Feature Plugins module

Others

  • Run PHPUNIT with wp-env
  • Jest Unit test for tracking.js
  • Added matrix strategy for PHP unit. (Notes: We can not run the WP tests with 5.6 since the latest WP version requires 7.2. PHP 8.2 and 8.3 require higher polyfills, which are not compatible with 8.0 or lower )

@Soare-Robert-Daniel Soare-Robert-Daniel self-assigned this Jun 25, 2024
Copy link

github-actions bot commented Jun 25, 2024

Plugin build for 4d9d200 is ready 🛎️!

Copy link
Collaborator

@carlalexander carlalexander left a comment

Choose a reason for hiding this comment

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

I've left a lot of notes. I think we should use this opportunity to shore up a few things related to testing. I've mentioned @selul a few times to see what he thinks.

Thank you for all your work! 🥳

key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}
restore-keys: |
${{ runner.os }}-composer-
- name: Install composer deps
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should still use setup-php here to make sure we can control the PHP version. We don't know how GHA will update it.

What does it run it against right now?

Comment on lines 22 to 32
- name: Get Composer Cache Directory
id: composer-cache
run: |
echo "::set-output name=dir::$(composer config cache-files-dir)"
- name: Configure Composer cache
uses: actions/cache@v1
with:
path: ${{ steps.composer-cache.outputs.dir }}
key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}
restore-keys: |
${{ runner.os }}-composer-
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe @selul can chime in as well, but I don't know if doing this is necessary anymore. I used to do that a long time ago especially with Composer v1 which was slower. I don't do that anymore.

I see this is in test-php as well. Maybe we could remove it there as well and make this a larger testing improvement pass.

Copy link
Contributor

Choose a reason for hiding this comment

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

It makes sense, I think this is from a few years back.

@@ -47,7 +47,7 @@ jobs:
- name: Setup PHP version
uses: shivammathur/setup-php@v2
with:
php-version: '7.2'
php-version: "7.4"
Copy link
Collaborator

Choose a reason for hiding this comment

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

@selul how would you feel about using this opportunity to switch this to a test matrix. We should run PHP tests against PHP 7.4, 8.0, 8.1, 8.2. and 8.3.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds like a great plan.

Comment on lines +1 to +3
.DS_Store
.idea
node_modules
.vscode
Copy link
Collaborator

Choose a reason for hiding this comment

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

We shouldn't add things related to the operating system or the IDE to .gitignore. Every developer should have a global .gitignore to take of that. I'll make a note in Slack 😃

.gitignore Show resolved Hide resolved
tests/e2e/specs/promotions.spec.js Show resolved Hide resolved
tests/e2e/specs/promotions.spec.js Show resolved Hide resolved
tests/e2e/specs/promotions.spec.js Show resolved Hide resolved
tests/e2e/specs/rollback.spec.js Outdated Show resolved Hide resolved
tests/e2e/specs/script-loader.spec.js Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants