Skip to content

Conversation

@noahtallen
Copy link
Contributor

@noahtallen noahtallen commented Jul 12, 2023

As I began working @tyxla's comment here (#79097 (comment)), I realized it'd be good to update various wp-env/ETK things (which I've been putting off for a while).

Proposed Changes

It's been a while since I touched wp-env in Calypso. This updates our config to make it compatible with new wp-env versions, which in turn means we're updating phpunit and setting a newer PHP version. As a result, there were a few other things to fix too.

Importantly, there are some features in ETK which were not written to work in local environments. In fact, they crash when not running on Automattic infrastructure. I decided to leave fixes for these out of the PR to make it less risky, but the result is that some pages of wp-env do not work. I will follow up with teams to try to get this resolved.

Local Testing Instructions

  1. Run docker
  2. cd apps/editing-toolkit
  3. Build the plugin (yarn build)
  4. After running composer install, run yarn test:php locally.

Wpcom testing instructions

  1. On your sandbox, run install-plugin.sh etk update-wp-env.
  2. Sandbox a test site and load the "new page" editor
  3. Verify the "page pattern picker" loads, and then use the three-dot info dropdown in the editor to check that the ETK version is the same as the one installed by install-plugin in step 1.
  4. Verify that the help center and whats-new still load on wpcom.

@noahtallen noahtallen requested a review from a team as a code owner July 12, 2023 00:46
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 12, 2023
@github-actions
Copy link

github-actions bot commented Jul 12, 2023

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PHP 8 does not allow you to access arrays directly if the index doesn't exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PHP 8 noticed these fields were undefined when going through other tests, which probably means the mock data wasn't fully created in the first place.

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 directory doesn't exist any more

composer.json Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The wp-env update basically meant that you should manage your own phpunit version instead of bundling it, so we do that here.

@noahtallen noahtallen requested a review from a team as a code owner July 12, 2023 00:53
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@noahtallen noahtallen requested review from a team and tyxla July 12, 2023 01:00
@matticbot
Copy link
Contributor

This PR modifies the release build for editing-toolkit

To test your changes on WordPress.com, run install-plugin.sh editing-toolkit update-wp-env on your sandbox.

To deploy your changes after merging, see the documentation: PCYsg-mMA-p2

@noahtallen noahtallen changed the title Update wp env Update wp-env and some ETK PHP fixes Jul 12, 2023
@noahtallen noahtallen force-pushed the update-wp-env branch 2 times, most recently from 39135aa to 9db7293 Compare July 14, 2023 23:13
@noahtallen noahtallen merged commit dcca5c0 into trunk Jul 17, 2023
@noahtallen noahtallen deleted the update-wp-env branch July 17, 2023 22:48
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 17, 2023
@noahtallen
Copy link
Contributor Author

I removed the risky aspects of this, so what remains is really basic test code (verified by CI), and wp-env configuration (verified by CI and myself). The other non-test PHP changes are basically just phpcbf autofixes.

After testing, I'm confident this is safe to merge 👍

@tyxla
Copy link
Member

tyxla commented Jul 19, 2023

This is some great work @noahtallen 🙌 The wp-env version was quite behind the latest one.

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.

4 participants