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

Updated wp-env To 8.0.0 #38440

Merged
merged 7 commits into from
May 26, 2023
Merged

Updated wp-env To 8.0.0 #38440

merged 7 commits into from
May 26, 2023

Conversation

ObliviousHarmony
Copy link
Contributor

Submission Review Guidelines:

Changes proposed in this Pull Request:

We are updating @wordpress/env to 8.0.0 in order to consume some upstream fixes.

  • wp-env no longer requires WSL to function.
  • Fixes to wp-env run that no longer require us to use quotes (and in fact, break, when we do)

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

  1. Run pnpm --filter=woocommerce env:test after updating.
  2. Run pnpm --filter=woocommerce test:unit:env and confirm they pass.
  3. Run pnpm --filter=woocommerce test:e2e-pw and confirm they pass.

@github-actions github-actions bot added focus: e2e tests Issues related to e2e tests focus: performance tests The issue/PR is release to performance testing. package: @woocommerce/onboarding issues related to @woocommerce/onboarding plugin: woocommerce Issues related to the WooCommerce Core plugin. labels May 24, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 25, 2023 •
edited
Loading

Test Results Summary

Commit SHA: c822909

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202611m 9s
E2E Tests1940010020414m 45s
To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

@ObliviousHarmony ObliviousHarmony marked this pull request as ready for review May 25, 2023 00:46
@ObliviousHarmony ObliviousHarmony requested review from a team and samueljseay and removed request for a team May 25, 2023 00:46
@github-actions
Copy link
Contributor

github-actions bot commented May 25, 2023 •
edited
Loading

Hi @jonathansadowski, @samueljseay, @rodelgc,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@github-actions
Copy link
Contributor

Hi @samueljseay,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@ObliviousHarmony
Copy link
Contributor Author

The code-analyzer is broken in this PR because of a breaking change in wp-env. Once it gets merged subsequent PRs won't have this problem.

lsinger
lsinger reviewed May 25, 2023
View reviewed changes
Copy link
Contributor

@lsinger lsinger left a comment

Choose a reason for hiding this comment

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

The changes look good to me, CI passes (with one expected and acceptable exception), but locally I always have one or two e2e tests fail even after retries. Not sure if that's acceptable / expected / OK so I'd like to hear the perspective from Solaris.

rodelgc
rodelgc reviewed May 26, 2023
View reviewed changes
plugins/woocommerce/.wp-env.json
"afterSetup": "./tests/e2e-pw/bin/test-env-setup.sh",
"lifecycleScripts": {
"afterStart": "./tests/e2e-pw/bin/test-env-setup.sh",
"afterClean": "./tests/e2e-pw/bin/test-env-setup.sh"
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for adding the afterClean scenario here!

@rodelgc
Copy link
Contributor

rodelgc commented May 26, 2023

Looking good! I ran the tests and GitHub workflows that are likely to be affected by this change.

  • ✅ Tested the upload-plugin.spec.js E2E test locally
  • ✅ Tested the update-woocommerce.spec.js E2E test locally
  • ✅ Launched wp-env environment using env:test:cot and ran basic.spec.js
  • ✅ Ran Smoke test release workflow on a clone of this branch. The "Test WooCommerce update" and "With (Plugin name)" jobs passed.
  • ✅ Ran Smoke test daily workflow on a clone of this branch. The jobs "E2E on nightly build" and "Smoke test with (plugin name) installed" passed.

Seems like there are conflicts in the PNPM lock file. I'm pre-emptively approving this PR in case the conflict isn't related to the changes here.

rodelgc
rodelgc previously approved these changes May 26, 2023
View reviewed changes
@ObliviousHarmony
Updated wp-env
bc45e77
@ObliviousHarmony
Updated wp-env 8.0.0 Compatibility
6ac1105
@ObliviousHarmony
Fixed afterSetup Replacement ENV Variable
8a7f9f7
@ObliviousHarmony
Changelog
dbadc04
@ObliviousHarmony
Fixed Test Warning
1ecc298
@ObliviousHarmony
Fixed Performance Tests
73e7bda
@ObliviousHarmony
Added afterClean
c822909 
When you run `wp-env clean` it will erase any options
that we set automatically. We need to set up the test
environment again if this was the case for clean-ness.
jonathansadowski
jonathansadowski approved these changes May 26, 2023
View reviewed changes
Copy link
Contributor

@jonathansadowski jonathansadowski left a comment

Choose a reason for hiding this comment

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

👍 looks good to me with the noted caveat about the code-analyzer failure being related to this change, which seems low-risk as this PR doesn't change any templates 😄

@ObliviousHarmony ObliviousHarmony merged commit 230bc04 into trunk May 26, 2023
@ObliviousHarmony ObliviousHarmony deleted the update/wp-env-8.0.0 branch May 26, 2023 22:48
@github-actions github-actions bot added this to the 7.9.0 milestone May 26, 2023
@ObliviousHarmony ObliviousHarmony self-assigned this Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lsinger lsinger lsinger left review comments

@jonathansadowski jonathansadowski jonathansadowski approved these changes

@rodelgc rodelgc rodelgc left review comments

@samueljseay samueljseay Awaiting requested review from samueljseay

Assignees

@ObliviousHarmony ObliviousHarmony

Labels
focus: e2e tests Issues related to e2e tests focus: performance tests The issue/PR is release to performance testing. package: @woocommerce/onboarding issues related to @woocommerce/onboarding plugin: woocommerce Issues related to the WooCommerce Core plugin.
Projects
None yet
Milestone
7.9.0
Development

Successfully merging this pull request may close these issues.
4 participants
@ObliviousHarmony @rodelgc @lsinger @jonathansadowski