New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The onActionDone
method should take TestRunErrorFormattableAdapter instance instead of Error (closes #4867)
#4847
Conversation
❌ Tests for the commit 6000808 have failed. See details: |
6000808
to
b990318
Compare
❌ Tests for the commit b990318 have failed. See details: |
onActionDone
onActionDone
❌ Tests for the commit da66e13 have failed. See details: |
da66e13
to
3a23d22
Compare
onActionDone
onActionDone
method should take TestRunErrorFormattableAdapter instance instead of Error (closes #4867)
❌ Tests for the commit 3a23d22 have failed. See details: |
❌ Tests for the commit 3a23d22 have failed. See details: |
✅ Tests for the commit 3a23d22 have passed. See details: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's possible, I propose to reorganize this code using the following idea:
- error adapters for action errors should be created into the 'executeAction' function
- error adapters for testRun errors (MissingAwaitError, UncaughtError) should be created into the '_executeTestFn'
❌ Tests for the commit f6e49d1 have failed. See details: |
I closed this version in favor of #4895 |
The array of adapters is created when the
executeTestFn
function catches the error. When it happens we make the screenshot (if screenshot-on-fails is enabled), create adapters and store them into thetestRun.errs
property. After that we passtestRun.errs
to thereportTestDone
method.Now we also need to pass the TestRunErrorFormattableAdapter to the
onActionDone
. So we need to create adapters on theexecuteAction
method. In addition, we need to make screenshot on theexecuteAction
method. So we need to catch the error onexecuteAction
, create adapter and screenshot and rethrow the error to stop test execution. However, if we rethrow the error we will catch it again onexecuteTestFn
.We know that some errors does not occur on the
executeAction
step, but occurs onexecuteTestFn
, i.e. error in custom user's code. Thus we somehow need to detect if the error already was processed, and if it was not, we need to process it. I think it's a very bad idea to create adapters and screenshots in two places.Moreover, we have one more layer of catching the errors -
wrapTestFunction
. Inside thewrapTestFuncion
we catch execution errors and missing awaits, then create theTestCafeErrorList
object and rethrow it.Thus, inside the catch block of
executeTestFn
we catch theTestCafeErrorList
object.I think that it's better to leave creating adapters and making screenshots on fails as is, in one place: in the
executeTestFn
catch block. So in this block I emit newexecution-error
event which can be handled inside theexecuteAction
method.