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

E2E test to cover the plugin's main functionality #3

Closed
wants to merge 24 commits into from
Closed

E2E test to cover the plugin's main functionality #3

wants to merge 24 commits into from

Conversation

meszarosrob
Copy link
Contributor

This PR provides E2E tests with Playwright and a GitHub Action to run it.

WordPress "core" is migrating slowly to this tool, and Gutenberg the Block Editor already uses this.

Feel free NOT to merge this! This might be some overhead that you don't want to maintain.


I didn't want to overcomplicate the setup with an environmental file for the database user, password, etc., so these are "hardcoded" in the docker-compose.yml. There's nothing really to hide here anyway.

For the same reason, I opted not to create an orchestration script and set up the WordPress installation directly in the workflow file.

In case of a failure, the "test results" are attached. These are basically screenshots that might give some additional clues. You can see one here https://github.com/meszarosrob/comment-saver/actions/runs/5348260503. What exactly went wrong can be found by inspecting the job.


While the docker-compose.yml is for the E2E, it's pretty generic, so it could easily be moved to the root and the paths modified. In case you want to have a ready-made setup for the future.


GitHub doesn't allow me to set #2 as the base, so some commits are from there. Once #2 is merged, this nota bene will no longer be relevant.

For simplicity, it uses only PHP 7.4. The WP PHPCS latest stable release does not support PHP 8 WordPress/WordPress-Coding-Standards#2035 (comment).

Switching to a matrix strategy to run multiple versions might make sense.
This returns false since WP version 4.5 https://github.com/WordPress/WordPress/blob/98ec0401b6491625aa4fd50feb67f4d595c6b255/wp-includes/deprecated.php#L3734-L3750

In theory, this is a breaking change, as everybody using some ancient WP will lose this feature.
The expected type is a string
@willnorris
Copy link
Owner

willnorris commented Jun 22, 2023

I've only quickly glanced at this PR, so just preliminary thoughts... while neat, I think this might be overkill for a plugin this simple, and not something I'm necessarily going to want to maintain longterm.

Edit: sorry.. I also just saw that you basically said the same thing in initial message above 😄

@willnorris
Copy link
Owner

#2 is merged, so you can rebase this now.

@meszarosrob
Copy link
Contributor Author

I've only quickly glanced at this PR, so just preliminary thoughts... while neat, I think this might be overkill for a plugin this simple, and not something I'm necessarily going to want to maintain longterm.

Edit: sorry.. I also just saw that you basically said the same thing in initial message above smile

Absolutely fair! 😅 Nothing can cause more headaches than something where you have to do npm install. ✌️

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.

None yet

2 participants