-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Split hooks and open hooks in ide from reporter #7821
Conversation
Thanks for taking the time to open a PR!
|
Test summaryRun details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
Marking ready for review, but still need to add e2e tests and decide the best way to handle a hook inside of a hook (which will fix the issue in the driver tests), for example: |
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.
The styling looks good to me. The Open in IDE button works and takes me where I would expect in the code.
I don't see before
and after
hooks being numbered. They are split correctly though. Any reason for this? I would probably expand the reporter tests to include some before
and after
hooks to also test this, looks like it's only testing beforeEach
and afterEach
now.
@jennifer-shehane Not intentional, small bug. Should be fixed now. |
Great! The after/before hooks are now properly numbered. Going to dismiss my review for a more thorough review from @chrisbrieding
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.
Look good - nice work!
const condenseHooks = (runnable, getHookId) => { | ||
const hooks = _.compact(_.concat( | ||
runnable._beforeAll, | ||
runnable._beforeEach, | ||
runnable._afterAll, | ||
runnable._afterEach, | ||
)) | ||
|
||
return _.map(hooks, (hook) => { | ||
if (!hook.hookId) { | ||
hook.hookId = getHookId() | ||
} | ||
|
||
if (!hook.hookName) { | ||
hook.hookName = getHookName(hook) | ||
} | ||
|
||
return wrap(hook) | ||
}) | ||
} | ||
|
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.
lodash fp ftw...
const condenseHooks = (runnable, getHookId) => {
return _
.chain([
runnable._beforeAll,
runnable._beforeEach,
runnable._afterAll,
runnable._afterEach
])
.compact()
.tap((hook) => {
if (!hook.hookId) {
hook.hookId = getHookId()
}
if (!hook.hookName) {
hook.hookName = getHookName(hook)
}
})
.map(wrap)
.value()
}
User facing changelog
Additional details
Required a decently sized rewrite to the way that the reporter runnables store handles hooks
How has the user experience changed?
Before:
After:
PR Tasks
cypress-documentation
?type definitions
?cypress.schema.json
?