-
Notifications
You must be signed in to change notification settings - Fork 659
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
Handle errors inside the request hook methods (close #3786) #3868
Conversation
❌ Tests for the commit 6ad8946 have failed. See details: |
src/errors/test-run/templates.js
Outdated
`), | ||
|
||
[TEST_RUN_ERRORS.requestHookNotImplementedError]: err => markup(err, ` | ||
You should implement the "${err.methodName}" method of the "${err.hookClassName}" class. |
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.
You should implement the "${err.methodName}" method in the "${err.hookClassName}" class.
src/errors/test-run/templates.js
Outdated
`), | ||
|
||
[TEST_RUN_ERRORS.requestHookUnhandledError]: err => markup(err, ` | ||
An unhandled error occurred inside the "${err.methodName}" method of the "${err.hookClassName}" class: |
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.
An unhandled error occurred in the "${err.methodName}" method of the "${err.hookClassName}" class:
✅ Tests for the commit c806d3d have passed. See details: |
e3a8307
to
98df355
Compare
❌ Tests for the commit 98df355 have failed. See details: |
❌ Tests for the commit 2395cd4 have failed. See details: |
❌ Tests for the commit 650c64f have failed. See details: |
@testcafe-build-bot retest |
❌ Tests for the commit 650c64f have failed. See details: |
❌ Tests for the commit 650c64f have failed. See details: |
✅ Tests for the commit 650c64f have passed. See details: |
@AndreyBelym @AlexKamaev @LavrovArtem you can review it. I will resolve conflict in the |
@@ -1,4 +1,4 @@ | |||
import { pull as remove } from 'lodash'; | |||
import { pull, remove, chain } from 'lodash'; |
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.
I don't really like pull
, because here we definitely have two versions of remove
, but since its lodash
naming let it be so
src/test-run/index.js
Outdated
_onRequestHookMethodError (event, hook) { | ||
let err = event.error; | ||
|
||
if (err instanceof RequestHookNotImplementedMethodError === false) { |
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.
Just proposal, maybe this way is more clear.
const isRequestHookNotImplementedMethodError = err instanceof RequestHookNotImplementedMethodError
if (!isRequestHookNotImplementedMethodError ) {
...
}
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.
Ok. I will fix it.
❌ Tests for the commit a075168 have failed. See details: |
@testcafe-build-bot retest |
❌ Tests for the commit a075168 have failed. See details: |
@testcafe-build-bot retest |
❌ Tests for the commit a075168 have failed. See details: |
❌ Tests for the commit a075168 have failed. See details: |
✅ Tests for the commit a075168 have passed. See details: |
❌ Tests for the commit 23daf08 have failed. See details: |
✅ Tests for the commit 4cd556f have passed. See details: |
@AndreyBelym @AlexKamaev @LavrovArtem
First - this PR, next - DevExpress/testcafe-hammerhead#2036
@VasilyStrelyaev check please messages in the error templates