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

Fix E2E test when using RC #2393

Merged
merged 5 commits into from
May 13, 2024
Merged

Fix E2E test when using RC #2393

merged 5 commits into from
May 13, 2024

Conversation

puntope
Copy link
Contributor

@puntope puntope commented May 8, 2024 •
edited
Loading

Changes proposed in this Pull Request:

When testing E2E for WC 8.9.0-rc.1 I saw the E2E tests were failing because the workflow was not able to deactivate WooCommerce.

After investigating. Seems like this is happening because the way we load and install WooCommerce in our wp-env. This PR updates the logic to install WC in the test-env-setup.sh file.

Replace this with a good description of your changes & reasoning.

Screenshots:

Before
Screenshot 2024-05-08 at 18 25 26

After
Screenshot 2024-05-08 at 18 25 51

Detailed test instructions:

  1. Don't checkout this PR
  2. Run npm run wp-env:up
  3. Run npm run -- wp-env run tests-cli -- wp plugin update woocommerce --version=8.9.0-rc.1
  4. See the operation failing.
  5. Checkout this PR
  6. Run npm run wp-env:up
  7. Run npm run -- wp-env run tests-cli -- wp plugin update woocommerce --version=8.9.0-rc.1
  8. See the operation succeeding.
  9. See workflow for E2E tests failing IN THE SETUP before this PR https://github.com/woocommerce/google-listings-and-ads/actions/runs/9001351242
  10. See them correct IN THE SETUP when using this PR https://github.com/woocommerce/google-listings-and-ads/actions/runs/9003604947

Additional details:

Changelog entry

Fix - E2E test when using RC

@puntope puntope changed the title Setup WooCommerce using sh Fix E2E test when using RC May 8, 2024
@github-actions github-actions bot added changelog: fix Took care of something that wasn't working. type: bug The issue is a confirmed bug. labels May 8, 2024
@puntope puntope self-assigned this May 8, 2024
@puntope puntope requested a review from a team May 8, 2024 14:31
@puntope puntope marked this pull request as ready for review May 8, 2024 14:34
martynmjones
martynmjones reviewed May 8, 2024
View reviewed changes
Copy link
Contributor

@martynmjones martynmjones left a comment

Choose a reason for hiding this comment

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

Hey @puntope, thanks for working on a fix to resolve the E2E test error.

The way WooCommerce is installed on the E2E environment was recently changed in #2362 to address an issue with the installation order and WooCommerce plugin dependency.

As these changes revert what was done is this not going to re-introduce problems?

@puntope
Copy link
Contributor Author

puntope commented May 9, 2024 •
edited
Loading

Hi @martynmjones thanks for the feedback. I thought the falling tests were because of another issue (I did a fix here https://github.com/woocommerce/google-listings-and-ads/compare/fix/e2e-tests)

But you're right that this might reproduce that issue you mentioned #2362 . So I tweaked the PR so I activated the extension also in wp-env

Now the tests failing should be only the ones here https://github.com/woocommerce/google-listings-and-ads/compare/fix/e2e-tests

Can we test again?

@puntope puntope mentioned this pull request May 9, 2024
Merged
@puntope puntope requested a review from martynmjones May 10, 2024 11:15
@eason9487
Merge pull request #2397 from woocommerce/dev/avoid-gha-malicious-input
ad30181 
Avoid running malicious inputs as shell commands in the GitHub Actions
@eason9487
Copy link
Member

eason9487 commented May 13, 2024 •
edited
Loading

Sorry, @puntope and @martynmjones
I forgot that #2397 depends on this PR and have merged it into this PR.

⚠️ Please note that the changes within the .github/workflows directory are not related to this PR.

Reverted in 7564eb8.

@eason9487
Revert "Merge pull request #2397 from woocommerce/dev/avoid-gha-malic…
7564eb8 
…ious-input"

This reverts commit ad30181, reversing
changes made to eb50e5d.

Ref: #2393 (comment)
@eason9487 eason9487 mentioned this pull request May 13, 2024
Merged
@eason9487 eason9487 added changelog: dev Developer-facing only change. and removed changelog: fix Took care of something that wasn't working. labels May 13, 2024
martynmjones
martynmjones approved these changes May 13, 2024
View reviewed changes
Copy link
Contributor

@martynmjones martynmjones left a comment

Choose a reason for hiding this comment

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

Thanks for adding the extra precaution against the plugin dependency error @puntope.

Tested again and all looks good now ✅

@puntope puntope merged commit bc7b0bf into develop May 13, 2024
2 checks passed
@puntope puntope deleted the fix/e2e-setup-wp-env branch May 13, 2024 13:05
@eason9487 eason9487 removed the type: bug The issue is a confirmed bug. label May 14, 2024
@puntope puntope mentioned this pull request May 14, 2024
Merged
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martynmjones martynmjones martynmjones approved these changes

Assignees

@puntope puntope

Labels
changelog: dev Developer-facing only change.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@puntope @eason9487 @martynmjones