-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
fix: disallow hook definitions in tests #9957
Merged
Merged
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
2983c35
fix: disallow hook defintions in tests
SimenB e8f74b7
Merge branch 'master' into disallow-hooks-in-tests
SimenB 35c23ca
Merge branch 'master' into disallow-hooks-in-tests
SimenB 7f38a26
push into unhandled
SimenB d612eab
update snap
SimenB 2e6b642
push error into current test
SimenB b212f99
fix type of testErrors
SimenB 861fbbf
another one
SimenB 7be0514
remove extraneous type
SimenB 011f593
update snap
SimenB File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
push into unhandled
- Loading branch information
commit 7f38a269a1fd18dd8bb111f52643228fe4ac595f
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,9 +39,12 @@ const eventHandler: Circus.EventHandler = ( | |
const {currentDescribeBlock, currentlyRunningTest} = state; | ||
|
||
if (currentlyRunningTest) { | ||
throw new Error( | ||
`Cannot nest a describe inside a test. Describe block "${blockName}" cannot run because it is nested within "${currentlyRunningTest.name}".`, | ||
state.unhandledErrors.push( | ||
new Error( | ||
`Cannot nest a describe inside a test. Describe block "${blockName}" cannot run because it is nested within "${currentlyRunningTest.name}".`, | ||
), | ||
); | ||
break; | ||
} | ||
|
||
const describeBlock = makeDescribe(blockName, currentDescribeBlock, mode); | ||
|
@@ -94,19 +97,21 @@ const eventHandler: Circus.EventHandler = ( | |
const {asyncError, fn, hookType: type, timeout} = event; | ||
|
||
if (currentlyRunningTest) { | ||
throw new Error( | ||
`Hooks cannot be defined inside tests. Hook of type "${type}" is nested within "${currentlyRunningTest.name}".`, | ||
state.unhandledErrors.push( | ||
new Error( | ||
`Hooks cannot be defined inside tests. Hook of type "${type}" is nested within "${currentlyRunningTest.name}".`, | ||
), | ||
); | ||
break; | ||
} else if (hasStarted) { | ||
state.unhandledErrors.push( | ||
new Error( | ||
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 need to mutate the message in |
||
'Cannot add a hook after tests have started running. Hooks must be defined synchronously.', | ||
), | ||
); | ||
} | ||
|
||
const parent = currentDescribeBlock; | ||
|
||
if (hasStarted) { | ||
asyncError.message = | ||
'Cannot add a hook after tests have started running. Hooks must be defined synchronously.'; | ||
state.unhandledErrors.push(asyncError); | ||
break; | ||
} | ||
const parent = currentDescribeBlock; | ||
|
||
currentDescribeBlock.hooks.push({asyncError, fn, parent, timeout, type}); | ||
break; | ||
|
@@ -116,14 +121,18 @@ const eventHandler: Circus.EventHandler = ( | |
const {asyncError, fn, mode, testName: name, timeout} = event; | ||
|
||
if (currentlyRunningTest) { | ||
throw new Error( | ||
`Tests cannot be nested. Test "${name}" cannot run because it is nested within "${currentlyRunningTest.name}".`, | ||
state.unhandledErrors.push( | ||
new Error( | ||
`Tests cannot be nested. Test "${name}" cannot run because it is nested within "${currentlyRunningTest.name}".`, | ||
), | ||
); | ||
break; | ||
} else if (hasStarted) { | ||
state.unhandledErrors.push( | ||
new Error( | ||
'Cannot add a test after tests have started running. Tests must be defined synchronously.', | ||
), | ||
); | ||
} | ||
if (hasStarted) { | ||
asyncError.message = | ||
'Cannot add a test after tests have started running. Tests must be defined synchronously.'; | ||
state.unhandledErrors.push(asyncError); | ||
break; | ||
} | ||
|
||
|
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.
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.
pushing into the
errors
array is essentially what happened before, except we don't go through the handler again into theerror
case