Skip to content

Automated Testing: Remove Puppeteer CI Job#59311

Merged
talldan merged 1 commit into
trunkfrom
remove/puppeteer-dep-and-ci
Apr 2, 2024
Merged

Automated Testing: Remove Puppeteer CI Job#59311
talldan merged 1 commit into
trunkfrom
remove/puppeteer-dep-and-ci

Conversation

@Mamaduka

Copy link
Copy Markdown
Member

What?

Now that Playwright migration is completed, we can safely remove Puppeteer and related.

Testing Instructions

CI should be green.

@Mamaduka Mamaduka added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label Feb 23, 2024
@Mamaduka Mamaduka self-assigned this Feb 23, 2024
@Mamaduka

Copy link
Copy Markdown
Member Author

I think this needs to become a peer dependency just like Playwright. @swissspidy, @gziolo what do you think?

"puppeteer-core": "^13.2.0",

@github-actions

github-actions Bot commented Feb 23, 2024

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Mamaduka <mamaduka@git.wordpress.org>
Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
Co-authored-by: talldan <talldanwp@git.wordpress.org>
Co-authored-by: gziolo <gziolo@git.wordpress.org>
Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org>
Co-authored-by: kevin940726 <kevin940726@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@Mamaduka Mamaduka force-pushed the remove/puppeteer-dep-and-ci branch from e76059c to 1ff8817 Compare February 23, 2024 11:44
@gziolo

gziolo commented Feb 23, 2024

Copy link
Copy Markdown
Member

I think this needs to become a peer dependency just like Playwright. @swissspidy, @gziolo what do you think?

"puppeteer-core": "^13.2.0",

As long as it works when using @wordpress/scripts alone which it should, it's fine with me.

@youknowriad

Copy link
Copy Markdown
Contributor

@Mamaduka can we remove the npm dependency entirely since we now recommend playwright, It can be a breaking change in the wp-scripts package but that seems fine.

@ellatrix ellatrix left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this all that needs to be removed? What about the e2e-tests package?

@Mamaduka

Copy link
Copy Markdown
Member Author

Is this all that needs to be removed? What about the e2e-tests package?

First we’ll need to relocate test plugins in different directory and then I guess deprecate the package. Not sure what’s the policy about that.

@talldan

talldan commented Feb 26, 2024

Copy link
Copy Markdown
Contributor

First we’ll need to relocate test plugins in different directory and then I guess deprecate the package. Not sure what’s the policy about that.

I think @wordpress/nux is a package that has been deprecated in the past. There was also an attempt to remove it in the past which didn't go so well as it's used by core. This one might be different as it's only a dev dependency. 🤔

@ntsekouras

Copy link
Copy Markdown
Contributor

@Mamaduka can we remove the npm dependency entirely since we now recommend playwright, It can be a breaking change in the wp-scripts package but that seems fine.

➕ to this.

@kevin940726

Copy link
Copy Markdown
Member

Another smaller thing is that we can now reference npm run test:e2e directly to npm run test:e2e:playwright in this repo. We can keep the test:e2e:playwright script for a while for muscle memory.

@talldan talldan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I noticed we're still running the puppeteer CI job. It takes five minutes even though it runs no tests 😆

Would love to see this PR merged to reduce our computations. ❤️

A small tracking issue for all the tasks associated with removing puppeteer would be great if anyone wants to volunteer to do that.

@talldan talldan force-pushed the remove/puppeteer-dep-and-ci branch from 1ff8817 to 34b004e Compare April 2, 2024 07:47
@talldan talldan enabled auto-merge (squash) April 2, 2024 07:48
@Mamaduka

Mamaduka commented Apr 2, 2024

Copy link
Copy Markdown
Member Author

Good point, @talldan!

I'll create a separate issue to remove Puppeteer from the scripts package. We can discuss the best options there.

@talldan talldan changed the title Project: Remove Puppeteer Automated Testing: Remove Puppeteer CI Job Apr 2, 2024
@talldan

talldan commented Apr 2, 2024

Copy link
Copy Markdown
Contributor

Decided to rebase this and it'll auto-merge when all tests pass.

@github-actions

github-actions Bot commented Apr 2, 2024

Copy link
Copy Markdown

Flaky tests detected in 34b004e.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8519245299
📝 Reported issues:

@talldan talldan merged commit 1da3209 into trunk Apr 2, 2024
@talldan talldan deleted the remove/puppeteer-dep-and-ci branch April 2, 2024 08:20
@github-actions github-actions Bot added this to the Gutenberg 18.1 milestone Apr 2, 2024
@talldan

talldan commented Apr 2, 2024

Copy link
Copy Markdown
Contributor

Oops, I forgot to add the co-authors. Sorry everyone 😞

cbravobernal pushed a commit to garridinsi/gutenberg that referenced this pull request Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants