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.

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

Migrate code block test case to playwright #40844

Closed

Conversation

pavanpatil1
Copy link
Contributor

What?

Migrate code-test.js to its Playwright version.

Why?

See #38570 for its background.

How?

See MIGRATION.md for migration steps.

Testing Instructions

Run npm run test-e2e:playwright test/e2e/specs/editor/blocks/code.spec.js

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label May 5, 2022
@github-actions
Copy link

github-actions bot commented May 5, 2022

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @pavanpatil1! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@skorasaurus skorasaurus added the [Type] Automated Testing Testing infrastructure changes impacting the execution of end-to-end (E2E) and/or unit tests. label May 5, 2022
@talldan
Copy link
Contributor

talldan commented May 6, 2022

Thanks for working on this @pavanpatil1!

Just a few things to fix in this PR, but on the whole looks good. I'll invite @kevin940726 for any further feedback.

Gutenberg uses the prettier code formatter, and it looks like the 'Static Analysis' check is failing because the newly added code isn't formatted. You can run npm run format to format files. For an individual file I think the command would be something like npm run format -- test/e2e/specs/editor/blocks/code.spec.js.

If you're using an editor like VSCode you can also install a prettier plugin and configure it format code automatically.

It also looks like there's a snapshot file used by the old test that needs to be deleted - specs/editor/blocks/__snapshots__/code.test.js.snap.

@talldan talldan requested a review from kevin940726 May 6, 2022 07:08
Copy link
Member

@kevin940726 kevin940726 left a comment

Choose a reason for hiding this comment

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

Apart from the minor issues Dan already mentioned, it mostly looks pretty good! I just have a small feedback to suggest inlining the clickBlockAppender util if possible.

} );

test( 'can be created by three backticks and enter', async ({page,pageUtils}) => {
await pageUtils.clickBlockAppender();
Copy link
Member

Choose a reason for hiding this comment

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

We usually want to limit the usage of e2e utils if we can inline them directly. If we're just creating a page, I believe we can just click on the appender button directly:

Suggested change
await pageUtils.clickBlockAppender();
await page.click( 'role=button[name="Add default block"i]' );

The same code has also been used in copy-cut-paste tests. This way we don't have to create the clickBlockAppender util (unless we really need it in other situations).

@pavanpatil1
Copy link
Contributor Author

Hi @kevin940726, I addressed all the shared feedback. However, the CI is failing on Static Analysis (Linting, License, Type checks...) / All (pull_request)`. I followed the steps mentioned here #40844 (comment). Formatted the code still it is failing.

@kevin940726
Copy link
Member

Thank you for working on this and sorry for the late reply 🙇 .

Unfortunately, it seems like this test had already been migrated in #41136 by @JustinyAhin. Normally, we would want to close the latest one but we forgot about that 😞 . We'll try to pay attention to this kind of problem more in the future. Sorry about it! 🙇

We're still very grateful for your work, and hope that you can start another PR soon next time ;)

@kevin940726 kevin940726 closed this Jun 7, 2022
@JustinyAhin
Copy link
Member

@pavanpatil1 I'm happy to do a pair coding session with you to work on a migrating couple of tests migration if that interests you :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [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.

5 participants