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

Migrate 'core settings' e2e tests to Playwright #57581

Merged
merged 4 commits into from
Jan 10, 2024
Merged

Migrate 'core settings' e2e tests to Playwright #57581

merged 4 commits into from
Jan 10, 2024

Conversation

fai-sal
Copy link
Contributor

@fai-sal fai-sal commented Jan 5, 2024

What

Part of #38851.

PR migrates core-settings.test.js e2e tests to Playwright.

Why

Testing Instructions

npm run test:e2e:playwright -- test/e2e/specs/editor/various/core-settings.spec.js
@fai-sal fai-sal changed the title Migrate/editor modes e2e test Jan 5, 2024
@Mamaduka Mamaduka added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label Jan 5, 2024
@Mamaduka
Copy link
Member

Mamaduka commented Jan 5, 2024

Thanks for contributing, @fai-sal!

Unfortunately, I cannot pull down this branch to test locally. Can make sure you're fork and this branch is up to date - https://developer.wordpress.org/block-editor/contributors/code/git-workflow/#keeping-your-fork-up-to-date?

P.S. I would recommend creating a new file for migration instead of moving the test file and then deleting the old test in a separate comment. This makes it more manageable to deal with merge conflicts.
@fai-sal
Copy link
Contributor Author

fai-sal commented Jan 5, 2024

Hi @Mamaduka, Thanks for the feedback!. I have synced the branch with trunk, Can you please retry pulling this branch to test locally.

If still no luck then I can create another PR following your recommended approach.
Mamaduka
Mamaduka reviewed Jan 9, 2024
View reviewed changes
test/e2e/specs/editor/various/core-settings.spec.js Outdated
Comment on lines 28 to 32
const optionsBefore = await getOptionsValues(
optionsInputsSelector,
admin,
page
);
Copy link
Member

Choose a reason for hiding this comment

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

Note: I'm not sure we can improve this helper. Maybe @kevin940726 has any suggestions.
Mamaduka
Mamaduka approved these changes Jan 10, 2024
View reviewed changes
Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

Let's merge this.

Thanks again, @fai-sal!
@Mamaduka Mamaduka merged commit 7c21100 into WordPress:trunk Jan 10, 2024
54 checks passed
@github-actions github-actions bot added this to the Gutenberg 17.5 milestone Jan 10, 2024
This was referenced Jan 10, 2024
Closed
Closed
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.
2 participants
@fai-sal @Mamaduka