Ensure scenario termination #4378
Conversation
|
Hey @kobenguyent I've find and fix that cause the issue #4197 that you talk about here I have added the test case here and it's ok |
| @@ -113,6 +113,9 @@ module.exports = function (config) { | |||
|
|
|||
| recorder.add('retryTo', async () => { | |||
| tryBlock(); | |||
| }).catch(err => { | |||
| console.error('An error occurred:', err); | |||
| done(null); | |||
There was a problem hiding this comment.
I'm curious if done() should be enough?
There was a problem hiding this comment.
the test case that i added passed with the resolve promise so i guess it's enough ^^
Just retryTo will be update with my other pr, so when the first it's merge, i will update this file
I think it's a good practice to add the catch block when we add something to the recorder i presume
There was a problem hiding this comment.
ah I meant done() instead of done(null)
There was a problem hiding this comment.
ah I meant
done()instead ofdone(null)
The done is the resolve of the promise and I get this type
and in the file, done function already call with done(null) 🤷♂️
|
as this also addresses the stale process, so I think we would combine them into a branch. Then provide a beta version to test it out. |
|
hi @Horsty80 would you mind checking the failed tests? I changed the base branch there are some conflicts, I tried to resolve them but some failed tests. |
Yes of course ! I know that will be conflict with my other PR ^^ |
Motivation/Description of the PR
scenario.jscodeAlso, we have updated
done()to prevent it from being called more than once inside Scenario.Possibly the root of this kind of error.
Error: done() called multiple times inApplicable helpers:
Type of change
Checklist:
npm test)