-
-
Notifications
You must be signed in to change notification settings - Fork 750
better hooks handling by submitting where test.failed occur #4782
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
Changes from 1 commit
09a7b34
5b4a9b7
06311e9
e8a184f
d6a25ce
ed9b476
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,11 +72,12 @@ module.exports = function (config) { | |
| return | ||
| } | ||
|
|
||
| event.dispatcher.on(event.test.failed, test => { | ||
| if (test.ctx?._runnable.title.includes('hook: ')) { | ||
| output.plugin('screenshotOnFail', 'BeforeSuite/AfterSuite do not have any access to the browser, hence it could not take screenshot.') | ||
| event.dispatcher.on(event.test.failed, (test, _err, hookName) => { | ||
| if (hookName == 'BeforeSuite' || hookName == 'AfterSuite') { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shall we also print something to acknowledge users that there is no browser available at this stage
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, I think this information is not important it would be valuable if user does some actions in a browser and wants screenshots to be stored but as they can't do actions in AfterSuite, they don't expect screenshots to be added |
||
| // no browser here | ||
| return | ||
| } | ||
|
|
||
| recorder.add( | ||
| 'screenshot of failed test', | ||
| async () => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -121,12 +121,13 @@ module.exports = function (config) { | |
| deleteDir(dir) | ||
| }) | ||
|
|
||
| event.dispatcher.on(event.test.failed, (test, err) => { | ||
| if (test.ctx._runnable.title.includes('hook: ')) { | ||
| output.plugin('stepByStepReport', 'BeforeSuite/AfterSuite do not have any access to the browser, hence it could not take screenshot.') | ||
| event.dispatcher.on(event.test.failed, (test, _err, hookName) => { | ||
| if (hookName == 'BeforeSuite' || hookName == 'AfterSuite') { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same aforementioned question |
||
| // no browser here | ||
| return | ||
| } | ||
| persist(test, err) | ||
|
|
||
| persist(test) | ||
| }) | ||
|
|
||
| event.dispatcher.on(event.all.result, () => { | ||
|
|
||
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 guess it shall be ===, shall not it?
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.
it doesn't matter but yes, can be