Add best practices section to test_utils readme#82393
Add best practices section to test_utils readme#82393alisonelizabeth merged 2 commits intoelastic:masterfrom
Conversation
|
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
sebelga
left a comment
There was a problem hiding this comment.
Thanks for putting this doc here @alisonelizabeth ! 👍
| }); | ||
| ``` | ||
|
|
||
| - **Do not use** using `nextTick()`, `waitFor`, or `waitForFunc` helpers in tests. These helpers use `setTimeout` underneath and add latency in the tests, especially on CI where a timeout (even of a few ms) can trigger a timeout error. These helpers will eventually be deprecated once existing tests has been updated. |
There was a problem hiding this comment.
Would there be any downside to adding console.warn calls to these functions? This will help people developing tests realize they shouldn't use these helpers. They'll emit in CI too but I don't see a downside to that.
There was a problem hiding this comment.
I thought about this as well. The one downside I can think of is nextTick is used a fair bit in some of the existing tests, so I could see it being a little noisy. I did a quick search and I don't see waitForFunc being used anywhere, so I think this one can be safely removed. waitFor is only used in remote clusters and CCR right now.
There was a problem hiding this comment.
I think our team is the only consumer of these functions so I thought that being all warned 😊 removes the need for a console.warn. The idea is to deprecate and remove those functions eventually.
This PR updates the
test_utilsreadme with a best practices section. This information was previously shared with the team by @sebelga in a different medium.