better hooks handling by submitting where test.failed occur#4782
Merged
better hooks handling by submitting where test.failed occur#4782
Conversation
Contributor
DavertMik
commented
Jan 25, 2025
DavertMik
commented
- Minor debug improvements to Playwright
- Added hookInfo to event if test failed in a hook
- fixed screenshot and stepByStep plugin to work in Before/After hooks
kobenguyent
reviewed
Jan 26, 2025
lib/plugin/screenshotOnFail.js
Outdated
| 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.
I guess it shall be ===, shall not it?
Contributor
Author
There was a problem hiding this comment.
it doesn't matter but yes, can be
lib/plugin/screenshotOnFail.js
Outdated
| 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.
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.
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
it would be confusing in this case to see that some screenshots are missing
(btw it is current behavior)
but as they can't do actions in AfterSuite, they don't expect screenshots to be added
lib/plugin/stepByStepReport.js
Outdated
| 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.
Same aforementioned question
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.