Fixed running retryFailedStep inside tryTo #4831
Conversation
| checks.setup = true | ||
| for (const helper of Object.values(helpers)) { | ||
| try { | ||
| if (helper._beforeSuite) await helper._beforeSuite(suite) | ||
| if (helper._before) await helper._before(test) | ||
| } catch (err) { | ||
| err.message = `${helper.constructor.name} helper: ${err.message}` | ||
| if (checks.setup instanceof Error) err.message = `${err.message}\n\n${checks.setup?.message || ''}`.trim() |
There was a problem hiding this comment.
out of curiosity, the line 135 defines as a boolean and then line 143 we assign an Error to it.
Shall we assign err.message to checks.setup?.message
There was a problem hiding this comment.
yes
the value of setup is boolean|| Error
if it is error, we pick a previous message and append to current error
if it is true but we fail, we write error
if it is true and never fail we have true
See line 170
| } catch (err) { | ||
| err.message = `${helper.constructor.name} helper: ${err.message}` | ||
| if (checks.teardown instanceof Error) err.message = `${err.message}\n\n${checks.teardown?.message || ''}`.trim() | ||
| checks.teardown = err |
lib/effects.js
Outdated
| return recorder.add( | ||
| sessionName, | ||
| () => { | ||
| recorder.session.start(sessionName) | ||
| store.tryTo = true | ||
| hasAutoRetriesEnabled = store.autoRetries |
There was a problem hiding this comment.
how about isAutoRetriesEnabled?
| @@ -484,7 +484,7 @@ class Playwright extends Helper { | |||
| this.currentRunningTest = test | |||
|
|
|||
| recorder.retry({ | |||
| retries: process.env.FAILED_STEP_RETRIES || 3, | |||
| retries: test?.opts?.conditionalRetries || 3, | |||
There was a problem hiding this comment.
as you touched this, what do you think if we store default retries = 3 in Store?
There was a problem hiding this comment.
then we need a better name for it...
this handles a very strange edge cases in Playwright / Puppeteer
I don't know what name in store is applicable for this and in which other contexts can be used
| } else { | ||
| args.push(JSON.stringify(arg).slice(0, 300)) | ||
| } else if (arg) { | ||
| args.push((JSON.stringify(arg) || '').slice(0, 300)) |
There was a problem hiding this comment.
is there any chance JSON.stringify(arg) is null or undefined then '' is used here?
There was a problem hiding this comment.
I'd rather not print undefined here...
Not sure about null
But I'd rather not, it's not readable...
Fixed regression when retryFailedStep was enabled inside tryTo block
Other changes
checkmessages