Experimenting integration testing best practices with RTL and msw #27027
Conversation
kevin940726
added
[Type] Enhancement
| @@ -36,7 +36,7 @@ export default function BlockParentSelector() { | |||
| parentBlockType: _parentBlockType, | |||
| firstParentClientId: _firstParentClientId, | |||
| shouldHide: ! hasBlockSupport( | |||
| parentBlockType, | |||
| _parentBlockType, | |||
There was a problem hiding this comment.
I discovered a bug when writing these tests!
|
Size Change: +35 B (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
kevin940726
force-pushed
the
try/edit-widgets-integration-testing
branch
from
0bb3168 to
053c4a4
Compare
|
@kevin940726 Seeing this just reminded me of an earlier PR - #18855. |
kevin940726
force-pushed
the
try/edit-widgets-integration-testing
branch
from
0a3f916 to
bd87ce8
Compare
kevin940726
requested review from
adamziel,
ajitbohra,
chrisvanpatten,
draganescu,
ellatrix,
gziolo,
nerrad,
noisysocks,
ntwb,
talldan and
tellthemachines
as code owners
packages/edit-widgets/src/index.js
Outdated
| @@ -23,24 +23,32 @@ import { create as createLegacyWidget } from './blocks/legacy-widget'; | |||
| import * as widgetArea from './blocks/widget-area'; | |||
| import Layout from './components/layout'; | |||
|
|
|||
| let hasInitialized = false; | |||
There was a problem hiding this comment.
we should add some comment about what is this here for.
There was a problem hiding this comment.
Hmm, I'm not sure what to add here. As the code itself is pretty self-explanatory 🤔? I most likely have a blind spot here though.
| import './mocks/server'; | ||
| import availableLegacyWidgets from './mocks/available-legacy-widgets'; | ||
|
|
||
| // We need this since some tests run over 5 seconds threshold. |
There was a problem hiding this comment.
This is very concerning that this test requires a longer timeout which is close to what we use with e2e tests.
All existing unit tests pass in 45-ish seconds on my machine:
Test Suites: 430 passed, 430 total
Tests: 2 skipped, 4631 passed, 4633 total
Snapshots: 197 passed, 197 total
Time: 43.828s, estimated 72s
It means that 10 integration tests that run above the existing timeout would run longer than 4 633 unit tests.
There was a problem hiding this comment.
Yeah, only the second test takes that long though. I think it might be an existing performance bug with our component but not a problem with the test itself. I'll try to do some more research to find out why it's so slow.
It should be the size of the package Now that I look at it, we might need to move |
|
This is really cool!
Do you have numbers on how long it takes an integration test to run vs an equivalent E2E test? When would/should one choose one over the other?
Is there an alternative to forking? How will we keep our fork up to date with upstream changes? |
|
@noisysocks Great questions!
I don't, for now 😅, since there aren't many e2e tests for the widgets screen either. But in theory, integration tests should almost always be faster than e2e tests. We're only testing a specific package using However, there's an integration test which is taking over 7s in this PR. I believe it should be a bug of our components though, no way it should take that long. Might have something to do with #26724, I'll do some more digging into this issue.
Popularized by Kent C. Dodds in his article "Write tests. Not too many. Mostly integration.", IMO we should be doing integration tests wherever possible. Sometimes integration tests won't be enough (like if we want to test real browsers' behavior or do visual testing), then we'll forward it to the much slower e2e tests.
We might be able to do some Jest specific config to transform "react" call to "@wordpress/element" automatically (or maybe it has already been done!?). Then we don't have to maintain the fork and just let the package manager handles it. I'll give it a try later! FWIW, most official testing libraries are essentially using the same way we do here though. See |
|
Doing integration tests with mocks doesn't sound compelling to me. The REST API can change and we'd not be catching bugs and e2e tests allow us to test way more (all the PHP code) which is very important for widgets and FSE (PHP unit tests are not enough). So yeah e2e tests are slower than tests mocking APIs but I disagree with us doing more and more integration tests instead of e2e tests. I'm not against experimenting with these but I'm concerned about both the false positives and the cost of maintenance. As you can read, I have a very different perspective https://riad.blog/2020/07/21/deleting-tests-is-a-best-practice/ |
|
Also we run Gutenberg e2e tests on Core for each release and it's a great way for us to validate that the integration in Core works just as well as in the plugin. We catch a number of missing backports by doing so. |
kevin940726
force-pushed
the
try/edit-widgets-integration-testing
branch
from
9123064 to
21ab6cd
Compare
|
@youknowriad I mostly agree that e2e tests let us catch more bugs and resemble the most closely to the way users use the software. However, IMHO, doing integration tests is still useful in some ways:
If you think about it, the integration tests introduced here is kind of like e2e tests for the package rather than for the whole app 😂 . |
|
For |
This is the most compelling argument for me. Having tests for packages is good but I don't think we should be mocking things. The problem here is that we're trying to test For me, at the moment we need mocking, the tests become less compelling. |
kevin940726
force-pushed
the
try/edit-widgets-integration-testing
branch
2 times, most recently
from
d49b259 to
56db4eb
Compare
kevin940726
force-pushed
the
try/edit-widgets-integration-testing
branch
4 times, most recently
from
d831486 to
70a7ff6
Compare
kevin940726
force-pushed
the
try/edit-widgets-integration-testing
branch
from
70a7ff6 to
f12afaa
Compare
|
👋 I don't have a whole lot to add to this conversation, but I want to highlight that, once we introduce systematic mocking, integration tests lose a lot of their essence as integration tests, and thus lose value. We tend to think of testing methods based on a spectrum: The danger with this is that, when we seek something less granular and slightly more high-level than unit tests, we conflate that with integration tests. In reality, if we're talking about testing within a package, it's not really a question of integration. Rather, what we often want is a kind of unit testing operating on something more dev-facing, like package APIs. Perhaps we could think of these exported functions as second-order units — it's still unit testing, but in this context the "unit" is larger. |
|
I just wanted to reshare some previous work in the same area to give you some more ideas. There is also #26184 opened by @talldan where he proposes end to end tests for the standalone block editor. I suggested that it could use Storybook as a testing target. Have you considered a similar approach for the edit widgets package? @ellatrix did some very promising explorations to write e2e like tests that use Storybook to run tests related to RichText functionality:
There were also explorations to use visual testing:
We have been playing with snapshot testing for components as well, You can check #18031 where Storyshots integration with Storybook was enabled. It was removed a few months later. |
I like to think of this as e2e tests, but the "end" here is the package users (developers or code that consumes this code) 😅. I'm somehow convinced that maybe I'm also convinced that maybe
I agree. But I also feel like sometimes when mocking is still necessary, we should be using something more low-level like I want to propose a universal way of mocking things with I also want to experiment a new way of querying and asserting things with Thx for everyone's helps! I'm going to close this and open different PRs for different experiments mentioned above. As it's unlikely that doing integration testing in |
Description
tdlr;
Most tests in Gutenberg are unit tests (run by Jest on each component / utility function) and e2e tests (run by Jest and Puppeteer on a real browser environment). We have a big chunk of tests that should just be integration tests but we often instead write them as e2e tests, which are very powerful but also very very slow. This PR tries to lay down the foundation of integration testing in this repo and also encourage best testing practices.
To begin with, we have to first set goals of what we're trying to do in our tests.
Goals
role, text, orlabel.With the goals outlined, we can come up with a better integration testing infastructure with the helps from some popular libraries.
React Testing Library (
RTL)RTL is already used in some of our unit tests. Introduced in #20428, it became the recommended way to write tests both in Gutenberg but also recommended in the official React doc. Using it in the integration testing forces us to write better tests which resembles the way the feature is actually used, satsifying both 1, 3, and 4 of the goals.
@testing-library/jest-domUsing it with RTL gives us a set of custom matchers that matches the usage of RTL. Instead of writing this every time.
We can now write this instead.
mswNetwork layer mocking solution. Instead of mocking
fetchdirectly in tests viajest.spyOnorjest.fn, we can now just define it globally once with mocked fixture data. Once setuped, any requests in the tests will directly get passed to the mock function, making mocking seamless.Though we are focusing on integration testing in this PR,
mswis not only useful in integration testing. In the future, we can also usemswfor some of our e2e tests, storybook, or even during regular development. Enable us to create a unified experience of network mocking among the project.How has this been tested?
Run the test with the following command
Not sure why it won't terminate after tests pass though, but it's already happening in master and in every test.
Types of changes
New feature
Checklist: