add @testing-library/react #3302
Conversation
- set custom testIdAttribute for getByTestId to work
| import Adapter from "@wojtekmaj/enzyme-adapter-react-17"; | ||
| import Enzyme from "enzyme"; | ||
| import { enableFetchMocks } from "jest-fetch-mock"; | ||
|
|
||
| Enzyme.configure({ adapter: new Adapter() }); | ||
|
|
||
| configure({ testIdAttribute: "data-test" }); |
There was a problem hiding this comment.
Do you know if this also maintains functionality for data-testid ?
There was a problem hiding this comment.
No. This will need to be removed once #3295 is merged to use the default test id ('data-testid').
There was a problem hiding this comment.
Let's not fragment this codebase further until we have a good plan about migrating the entire codebase. This has been discussed a number of times already which you wouldn't be aware of @petermakowski, but we've agreed not to do this until someone comes up with a good plan for using RTL without the poor testing practices it encourages.
@huwshimi Couldn't agree more that we shouldn't start the migration without planning! But this PR is only adding testing-library/react as a dependency with basic setup - I'm not introducing anything controversial here. AFAIK everybody's on board with the idea that we're gonna have to stop using enzyme and replace it with testing-library eventually. Unless you have concerns with the incremental approach itself? Could you point me to where you had the discussion about RTL, I'd like to better understand your concerns around it. |
|
@huwshimi I'd personally find it very useful to have RTL as a dependency on master. Would allow to experiment easily with various ways of testing in the repo as I go - without having to constantly switch branches - this would help inform the planning and our approach. Just to reiterate - no RTL test will be written before having a plan that everybody's had a chance to contribute to and approved. |
hatched
dismissed
huwshimi’s stale review
discussions have been made and plans in place.
|
@petermakowski and I have had a discussion about a plan to migrate the tests. But in any event this should land to allow him to carry on with the test investigations. |
|
@petermakowski can you update this to master and ensure a passing CI run |
I think if this is just going to be for investigation that it might make more sense for it to be done in a fork, rather than having to maintain extra deps in master. |
Thanks for understanding @petermakowski. We started a document to begin the discussion: https://docs.google.com/document/d/1ysVlPByzVY-NBUjGic1WupHojUl30P6cf3Xe-OVRRPg/edit#. We haven't really got a good plan figured out for how to do good unit testing with RTL rather than the integration/accessibility/bdd style that RTL promotes. We also need to figure out a good plan for how/when to do such a large migration. Probably after the existing migrations are completed (Angular JS being the main piece in progress). |
|
Thanks for explaining @huwshimi As part of the investigation I created an epic to track this migration with a full list of files: #3318 Feel free to add more tasks. Let's make sure we address all concerns in the document https://docs.google.com/document/d/1ysVlPByzVY-NBUjGic1WupHojUl30P6cf3Xe-OVRRPg |
Done
QA
MAAS deployment
To run this branch you will need access to one of the following MAAS deployments:
Running the branch
You can run this branch by:
QA steps
Fixes
Fixes: # .
Launchpad issue
Related Launchpad maas issue in the form
lp#number.Backports
In general, please propose fixes against master rather than release branches (e.g. 2.7), unless the fix is only applicable for that specific release. Please apply backport labels to the PR (e.g. "Backport 2.7") for the appropriate releases to target.
Only bug and security fixes should be backported, new features should only land in master.