[RFC] Add docs for snapshot testing in Playwright e2e tests

[kevin940726]

What?

This PR does a few things:

1. Propose an article about overusing snapshots.
2. Add a new test util called editor.getBlocks().
3. Rearrange and docs and move them to /docs/contributors/code/e2e/

Why?

I'm seeing that snapshots are being misused in several places. I think an official guide on when and how to use snapshots would be a good idea. Future PRs can link to that document during reviews.

I'd expect things like this would happen more often given that we're in the middle of the discussion about best practices in e2e testing. Hence, I create an e2e folder to put all these RFC-like documents there.

How?

📝

[codesandbox]

  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

[Mamaduka]

Should we also mention serialized content assertions as well?

Getting the serialized expected results are easier by switching to the code editor.

await expect.poll( editor.getEditedPostContent )
			.toBe( `<!-- wp:paragraph -->
<p>1</p>
<!-- /wp:paragraph -->` );

[kevin940726]

Good idea! I think we should be using editor.getBlocks in these cases too, for the same reason as snapshot testing.

[Mamaduka]

I think I prefer to use serialized inline assertions for simple text blocks. Let's document both use variations, and we can choose depending on the test case. Unless you think this would make standardizing the tests harder.

P.S. I might change my mind when I use editor.geBlocks and toMatchObject more often.

[kevin940726]

I added a section about "snapshot variants". LMK what you think!

[kevin940726]

Oops, didn't see your last comment.

I think I'd prefer getBlocks wherever possible. But it's good because this is what this RFC is for 😆 . Do you have a concrete example of where you think serialized blocks make more sense?

[kevin940726]

I'm not denying HTML is readable, just that it might not be the best tool for the job. Imagine when the sole purpose of a test is to check for the content of a single column in the table block. Instead of comparing the full HTML of the table block, checking only the column makes the test easier to follow and review. It depends on the tests and their focus, but I often find comparing the full HTML in a snapshot out of focus. I rarely know what I should be looking for in those HTML when everything is equally important. 😅

[ellatrix]

That's where we disagree I think. I'd agree when creating unit tests, but with e2e test I think it's good to check the whole content for any unintended side effects.

[kevin940726]

I see! I guess my point is that we already have dedicated tests for that, and there are better ways than outputting the full HTML markup to whoever is reading the code.

May I suggest a middle ground of using the diff snapshot trick for this?

// Insert the block...

const beforeContent = await editor.getEditedPostContent();

// Do something in the test...

const afterContent = await editor.getEditedPostContent();
expect( diff( beforeContent, afterContent ) ).toMatchInlineSnapshot();

I'm perfectly fine with this technique as it also encourages minimal atomic assertions. It's less readable without any comment than explicit assertions though, but easier to write. This way, we get to test the full content without having to sacrifice readability with long dreadful snapshots. The coverage for beforeContent should already be covered in "Full post content test fixtures" so we don't have to check for that again. Would this sound like a better alternative to you?

[ellatrix]

Interesting. What diff function is that? We can try it for sure.

The coverage for beforeContent should already be covered in "Full post content test fixtures" so we don't have to check for that again.

I'm not talking about the initial content, but rather content after edits or other actions.

[kevin940726]

There are a lot of diff functions out there that we can use. The most common one might be the one from Jest: jest-diff.

I'm not talking about the initial content, but rather content after edits or other actions.

Yep, that's why I said we don't need to check the output of the initial content. We just want to check the "diff" between the initial one and the content after editing. I mentioned this just to note that we aren't sacrificing coverage for readability.

[Mamaduka]

Observation: The "Best practices" individual sections aren't massive, and details/summary usage might make it less readable. What do you think?

[kevin940726]

Yeah, it seems like GitHub will render large margins between these details tags. I'm fine either way, my main concern is that it might make the "Common pitfalls" section less discoverable.

[Mamaduka]

Maybe we can add ToC for level 2 headings? It should make it easier to scan what's in the document.

[kevin940726]

True! I considered that too. We'd have to make sure future revisions will reflect the TOC though. I wonder if there's already a linter or something similar in place so that we don't have to introduce it in this PR 🤔.

[Mamaduka]

I found the ToC example here https://developer.wordpress.org/block-editor/contributors/code/react-native/integration-test-guide/, but it's manually maintained.

[glendaviesnz]

I have made some minor grammar suggestions. It seems like this is in a reasonable place to merge and iterate on if needed.

[Mamaduka]

I agree with @glendaviesnz. Let's merge and iterate.

[kevin940726]

@ellatrix Pinging you again in case you missed it ❤️ .

[ellatrix]

Something what we discussed that I find missing here is about doing things in e2e tests programmatically. Would be good if we have a section that says to avoid shortcutting actions programmatically except for at the beginning or end of the test, to set up state or verify state. If you find yourself altering state programmatically somewhere in the middle, it's probably a sign that the test is too big.

[kevin940726]

Something what we discussed that I find missing here is about doing things in e2e tests programmatically.

I have that in mind! I think it deserves another PR and another article though. I believe we still have some disagreement about that and it would be nice to start a new conversation there ❤️ !

[ellatrix]

Let's try it

[Mamaduka]

@kevin940726, I think we want to add these new docs to manifest.json, so they appear in the handbook. Currently, the Playwright guide link mentioned in testing overview docs leads to 404 - https://developer.wordpress.org/block-editor/contributors/code/testing-overview/.

[kevin940726]

@Mamaduka Oh yeah, you're right! I opened #48447 for this.