Automated Testing Guidelines for Gutenberg

[BE-Webdesign]

In conversation with jnylen, and youknowriad, there are definite grey areas around unit testing for Gutenberg. How should the assertions be styled? What conventions should be used? What should we test, and is markup testing useful? These are all concerns that should probably be ironed out into some sort of guideline so that testing can be done more streamlined and hopefully be consistent across contributors. Add your thoughts in the section below.

[BE-Webdesign]

Testing components can be viewed as somewhat useless in React as they are somewhat just supposed to work. Often times, I don't write tests for my view layer at all, but have found it is somewhat useful for tracking any regressions across upgrades of React, style sheets, etc. as a project evolves. Fully testing react-redux component bindings like connect() is pretty involved as well, for something that should just work. I am usually all for 100% coverage, but it is also possible in the context of Gutenberg and its current state, that going the full 100% coverage is not necessary.

Here is my list of priorities in unit testing.

1. Testing any framework free logic in Gutenberg.
2. Testing Redux state management.
3. Testing component instance methods.
4. Testing rendering of components.
5. Testing react-redux bound components.

[youknowriad]

Thanks again for the work you're doing on unit tests. I mostly agree with all these priorities. I do feel that fully testing the components (especially the rendering) is not necessary at the moment because these components are often subject to change.

---

That said, having more tests is better than having fewer tests, and if a test is problematic, it's easier to drop than having a missing test.

[BE-Webdesign]

That said, having more tests is better than having fewer tests, and if a test is problematic, it's easier to drop than having a missing test.

Yeah, I figured we could just drop them if they start to be a pain. For now I will chop that list at number 3 then probably. Sound good?

[nylen]

In general I am less worried about the specific conventions for tests than just that we have them in place. I agree the more the better.

I think there are other types of tests that will be important to add. I'll try to list all the kinds I can think of here.

• Unit tests for Redux reducers
• Unit tests for Redux actions
• Unit tests for Redux selectors
• Unit tests for Redux effects (pretty new; limited coverage)
• Unit tests for API calls and other side-effects
• Unit tests for React components (in progress, Unit Testing Components #641)
• Unit tests for block registration and other API functions (mostly present AFAIK)
• Unit tests for parsers, including the actual tree of content received from the parser (see Deleting a closing block comment in "Text" mode #844 and Parser: Revert to PEG parser by default #947)
• Integration tests for parsing/serialization of real blocks (Add full-content tests for parsing + blocks list + serialization #849)
• Integration tests for parsing/serialization of real blocks, ensuring similar behavior between both client and server (see Attributes on a block can't be camelCase #890, Parser: Use an HTML like attribute syntax for comment attributes #626 (comment) for example)
• Integration tests for basic UI loading, script registration, etc (see Add keycodes file #940 (comment) for an example about wp.utils)
• Integration tests for UX cases like inserting, manipulating blocks (would have caught the bug in Blocks: Fix inserting image blocks #962 for example. This can get pretty complicated, but we can look at https://github.com/Automattic/wp-e2e-tests for inspiration.)
• Running (selected?) tests against real browsers, probably using Webdriver. Similar to how core's QUnit test suite works.
• Automated tracking of performance (memory and CPU time); Webpack build sizes; etc. See Attempting to load Redux devtools freezes the page #919 (comment)
• Code coverage reporting.

[aduth]

Fully testing react-redux component bindings like connect() is pretty involved as well, for something that should just work.

Part of what I like about the separation between the connect call and the component itself is the distinction of presentational and container components, specifically in reflecting that the presentational component doesn't really care where its props come from; the connect is merely one possibility. In this regard, I don't know that it's necessary to test the connected component, but rather the presentational one only with simulated props that we'd otherwise expect from the connect call. This might also push us to building components which aren't so heavily bound to the shape of application state.

[nylen]

Fully testing react-redux component bindings like connect() is pretty involved as well, for something that should just work.

I'm not super convinced that we need to do this either, as we already encourage state access only through selector functions, which are tested. However I'm always happy to review PRs that add more tests. :)

[BE-Webdesign]

rather the presentational one only with simulated props that we'd otherwise expect from the connect call. This might also push us to building components which aren't so heavily bound to the shape of application state.

So should we export the presentational component, mapStateToProps, and mapDispatchToProps, for testing, and let the connect() component just be?

[aduth]

imo, we'd export a named export for the component, to be used in testing, then the default export is the connected component intended to be rendered by the UI. To @nylen's point, don't bother with testing connected component since we test selectors instead.

export function MyComponent() {}

export default connect()( MyComponent );

We could make these two separate files, each with their own default export. In fact, this is a common pattern with container and presentational components. But I find it nigh discourages developers from connecting their components with sufficient granularity vs. creating overly broad container components. More pragmatic, if you would.

[BE-Webdesign]

But I find it nigh discourages developers from connecting their components with sufficient granularity vs. creating overly broad container components. More pragmatic, if you would.

You lost me 😓. Should we make the separate files, or is it not valuable enough to warrant the extra separation? I am having difficulty understanding what you want me to do.

[aduth]

Component in a single file, with named presentational export for testing, default connected export for UI.

[mtias]

Going to close this as it doesn't seem we have immediate actionable items. Feel free to reopen if you want to continue discussing.