[Jest Circus] Make hooks in empty describe blocks error #6320

captbaritone · 2018-05-27T21:50:34Z

captbaritone · 2018-05-27T21:52:00Z

integration-tests/__tests__/__snapshots__/empty-describe-with-hooks.test.js.snap

+
+  ● Test suite failed to run
+
+    thrown: \\"Invalid: beforeEach() in a describe block with no tests.\\"


@SimenB This is using the same formatting as user errors. I'm not sure if these errors should have the thrown: prefix. Thoughts?

Also, not sure about the wording here. Got any suggestions for other wording that would be more in-line with other Jest errors?

Yeah, the thrown part is a bit heavy handed. Not sure how to best differentiate. Maybe you can just copy the stack from asyncError and append your own message? Like state.unhandledErrors.push({message: myMessage: stack: asynError.stack}) or something?

The "thrown" is actually added in _formatError: https://github.com/facebook/jest/blob/master/packages/jest-circus/src/utils.js#L315

You can probably just do asyncError.message = 'my message' and push it into errors, and it'll be correct, without the thrown: part

captbaritone · 2018-05-27T21:52:29Z

integration-tests/__tests__/__snapshots__/empty-describe-with-hooks.test.js.snap

+      11 | 
+      12 | describe('another block with tests', () => {
+
+      at packages/jest-circus/build/index.js:33:17


@SimenB Is there anything special that I need to do to filter out this line of the stack trace?

I was a bit lazy in #6281 and didn't add all the Error.captureStackTraces I should. I just added it to test, should be added to all hooks as well.

A gotcha is that it only removes a single frame, so when we have _createHook or whatever the function is called, you need to move the error creation one level up to remove the correct frame

SimenB

So happy about this!

Does this deserve a changelog entry? This is a new feature of Circus, not just feature parity with jasmine

captbaritone · 2018-05-27T22:04:23Z

@SimenB It should probably be a part of the Jest Circus changelog entry. Any idea where to write that in the mean time?

SimenB · 2018-05-27T22:05:49Z

something like

### Features

* `[jest-circus]` Make hooks in empty describe blocks error ([#6320](https://github.com/facebook/jest/pull/6320))

?

aaronabramov

this is amazing!

now what about this case? 😛

test('lol', () => {
  test('lmao', () => {});
});

i've seen in somewhere in our codebase

aaronabramov · 2018-05-27T22:24:12Z

integration-tests/empty-describe-with-hooks/__tests__/hook-in-empty-describe.test.js

@@ -0,0 +1,14 @@
+/**


don't forget to move these to e2e :)

aaronabramov · 2018-05-27T22:25:38Z

packages/jest-circus/src/event_handler.js

+        currentDescribeBlock.hooks.forEach(hook => {
+          state.unhandledErrors.push([
+            `Invalid: ${hook.type}() in a describe block with no tests.`,
+            hook.asyncError,


i'm so happy that this will actually show where the hook is defined

SimenB · 2018-05-27T22:37:07Z

@aaronabramov the test in test thing can be even worse, FB apparently needs hooks inside tests: #6098.

SimenB · 2018-05-27T23:13:30Z

packages/jest-circus/src/index.js

+  if (Error.captureStackTrace) {
+    Error.captureStackTrace(asyncError, hookFn);
+  }
+  dispatch({asyncError, fn, hookType, name: 'add_hook', timeout});


it was returned previously? not sure if it matters

dispatch() returns void.

SimenB · 2018-05-27T23:14:15Z

e2e/__tests__/empty-describe-with-hooks.test.js

+test('hook in empty describe', () => {
+  const result = runJest(dir, ['hook-in-empty-describe.test.js']);
+  expect(result.status).toBe(1);
+  expect(extractSummary(result.stderr)).toMatchSnapshot();


missing the snapshots?

Whoops. Got lost in the move to e2e. Added.

SimenB · 2018-05-27T23:21:01Z

packages/jest-circus/src/event_handler.js

+      if (!describeBlockHasTests(currentDescribeBlock)) {
+        currentDescribeBlock.hooks.forEach(hook => {
+          state.unhandledErrors.push([
+            `Invalid: ${hook.type}() in a describe block with no tests.`,


if you do

hook.asyncError.message = `Invalid: ${hook.type}() in a describe block with no tests.`

I think the message will be without the extra quotes, and without thrown

cc417a2 Works perfectly. Thanks!

codecov-io · 2018-05-27T23:24:53Z

Codecov Report

Merging #6320 into master will decrease coverage by 0.09%.
The diff coverage is 30%.

@@            Coverage Diff            @@
##           master    #6320     +/-   ##
=========================================
- Coverage   63.86%   63.77%   -0.1%     
=========================================
  Files         228      228             
  Lines        8699     8713     +14     
  Branches        3        3             
=========================================
+ Hits         5556     5557      +1     
- Misses       3142     3155     +13     
  Partials        1        1

Impacted Files	Coverage Δ
.../src/legacy_code_todo_rewrite/jest_adapter_init.js	`0% <0%> (ø)`	⬆️
packages/jest-circus/src/event_handler.js	`12.85% <0%> (-0.78%)`	⬇️
packages/jest-circus/src/index.js	`52.17% <38.46%> (-9.37%)`	⬇️
packages/jest-circus/src/utils.js	`21.73% <50%> (+0.5%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c78f3d...dbadab9. Read the comment docs.

captbaritone · 2018-05-27T23:35:04Z

@aaronabramov It looks like tests within tests are explicitly supported. In fact, that's how test.each works. cc @mattphillips

Fixes jestjs#6293

mattphillips · 2018-05-29T11:29:07Z

@captbaritone test.each doesn't nest tests it take the global test function as a callback and forEachs over the data calling the test callback making each test a sibling.

test.each function is just sugar for forEach 😄

github-actions · 2021-05-12T13:15:21Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

captbaritone commented May 27, 2018

View reviewed changes

SimenB approved these changes May 27, 2018

View reviewed changes

facebook-github-bot added the cla signed label May 27, 2018

aaronabramov approved these changes May 27, 2018

View reviewed changes

captbaritone force-pushed the error-on-empty-each branch from f5a9152 to b689d1f Compare May 27, 2018 23:02

SimenB reviewed May 27, 2018

View reviewed changes

captbaritone force-pushed the error-on-empty-each branch from b3c78bc to 6dd3919 Compare May 27, 2018 23:49

captbaritone added 8 commits May 28, 2018 11:18

[Jest Circus] Make hooks in empty describe blocks error

62a5d1d

Fixes jestjs#6293

Prettier

78b55cf

Document hooks in empty describe blocks in CHANGELOG.md

5599278

Trim stack trace for hooks in empty describe blocks

4d7aa3f

Move to e2e

8007555

Add missing snapshots

dcfb576

Improve error message

26c090f

Add test asserting we can have hooks in blocks with skipped tests

dbadab9

captbaritone force-pushed the error-on-empty-each branch from 6dd3919 to dbadab9 Compare May 28, 2018 10:19

Merge branch 'master' into error-on-empty-each

47f8a71

cpojer merged commit f1426ce into jestjs:master May 30, 2018

dependencies bot mentioned this pull request Nov 1, 2018

Update jest in / from 22.1.4 to 23.6.0 leonardofaria/leozera-ui#250

Closed

snyk-bot mentioned this pull request Mar 17, 2020

[Snyk] Upgrade expect from 23.0.0 to 23.6.0 yowainwright/react-workshop#8

Merged

github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Jest Circus] Make hooks in empty describe blocks error #6320

[Jest Circus] Make hooks in empty describe blocks error #6320

captbaritone commented May 27, 2018

captbaritone May 27, 2018

SimenB May 27, 2018 •

edited

captbaritone May 27, 2018

SimenB May 27, 2018 •

edited

captbaritone May 27, 2018

SimenB May 27, 2018 •

edited

SimenB left a comment •

edited

captbaritone commented May 27, 2018

SimenB commented May 27, 2018

aaronabramov left a comment

aaronabramov May 27, 2018

aaronabramov May 27, 2018

SimenB commented May 27, 2018

SimenB May 27, 2018

captbaritone May 27, 2018

SimenB May 27, 2018

captbaritone May 27, 2018

SimenB May 27, 2018

captbaritone May 27, 2018

SimenB May 27, 2018

codecov-io commented May 27, 2018 •

edited

captbaritone commented May 27, 2018

mattphillips commented May 29, 2018

github-actions bot commented May 12, 2021


		● Test suite failed to run

		thrown: \\"Invalid: beforeEach() in a describe block with no tests.\\"

[Jest Circus] Make hooks in empty describe blocks error #6320

[Jest Circus] Make hooks in empty describe blocks error #6320

Conversation

captbaritone commented May 27, 2018

Choose a reason for hiding this comment

SimenB May 27, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB May 27, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB May 27, 2018 • edited

Choose a reason for hiding this comment

SimenB left a comment • edited

Choose a reason for hiding this comment

captbaritone commented May 27, 2018

SimenB commented May 27, 2018

aaronabramov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB commented May 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented May 27, 2018 • edited

Codecov Report

captbaritone commented May 27, 2018

mattphillips commented May 29, 2018

github-actions bot commented May 12, 2021

SimenB May 27, 2018 •

edited

SimenB May 27, 2018 •

edited

SimenB May 27, 2018 •

edited

SimenB left a comment •

edited

codecov-io commented May 27, 2018 •

edited