doc(proposal): flaky
#4
Conversation
|
I recall this issue being opened a while back, and if I remember correctly, we didn't want to add a pattern that promotes not fixing flaky tests Edit: |
|
Thanks Aviv! Moshe was on the team call I proposed this (I'm pretty sure he was still there—he dropped for a few seconds had then had to leave a few minutes early), and he's the only quasi-dissenter in nodejs/node#48754 I see comments that there was a second dissenter on your PR, but I don't see who it was. EDIT: oh, actually Moshe read the meeting notes too (or at least indicated he had) and it's called out twice in them. So I think we're okay to proceed? Perhaps we can resurrect your PR or, if not entirely, leverage parts of it (with Co-author credit of course). Yours might have a very different design (I haven't checked yet), and maybe Moshe liked the broad-strokes of this that were highlighted in the meeting . |
|
I have objected to this (not the exact same propasal) in the past but I wont block if someone wants to pick it up again |
|
Thanks Moshe! I read through your past concerns, and I agree with the sentiment (to not encourage users to ignore problems). Can you think of anything that we could incorporate into this that would limit that or encourage fixing the flakiness? Perhaps something mildly unpleasant that one could ignore but is ever-so-slightly irksome (just enough that one could deal with for now but not forever). |
|
Oow, what if it doesn't count towards code coverage? (Maybe too difficult to facilitate in implementation though 🤔) |
|
I guess the current situation is limiting, but allowing to use retries - with user land retries that are pretty trivial to use? |
| it('may also take several times', { flaky: 100 }, () => {…}); | ||
| ``` | ||
|
|
||
| When `flaky` is `true`, the test harness re-tries the suite or test-case up to the default number of times (eg 20), inclusive. |
There was a problem hiding this comment.
I would say that if we provide a default, we should also provide a way to define a global default. It could be a "phase 2" implementation, but I would suggest thinking about it!
(In any case I would also suggest defining an initial default of around 5 👀 )
There was a problem hiding this comment.
Shall we do via the usual ENV + flag + configFile? I was hoping to crowd-source ideas for that 🙂
|
@MoLow sorry, I don't understand your comment. |
cc @vespa7
Outstanding questions: