jest-circus: throw if a test / hook is defined asynchronously

[jeysal]

Summary

This fixes the behavior when defining tests / hooks asynchronously in jest-circus.
Previously outside of a describe block (in the root describe block) the tests would be accepted and run.
Previously inside a describe block this error would be thrown:

But it was also possible to time it so that the test declared asynchronously in the describe block would become a test of the root describe block.

With this PR async definitions anywhere will throw:

Test plan

circus-only e2e test added

[jeysal]

cc @aaronabramov since this touches circus

[SimenB]

not for this PR, but we should use tsdoc for these so they show up when using autocomplete

[SimenB]

This is great, thank you!

Needs to make the test non-flaky and add a changelog entry 🙂

[jeysal]

Not sure if this is a fix or feature for the changelog 🤔

[SimenB]

Not sure if this is a fix or feature for the changelog 🤔

ergh... fix I'd say. It might have been working by accident before, and probably flaky depending on how much resources your computer had. So it's just an explicit error in the case of user error

[jeysal]

beforeEachQueue e2e test is failing

[jeysal]

Apparently defining hooks in tests is supported? #6098

[SimenB]

@mjesun @rubennorte did you ever manage to migrate away from those funky tests?

[jeysal]

@scotthovestadt perhaps you could also answer this using fbcodesearch or whatever it's called?
TLDR does FB still do {before,after}{Each,All} inside of it/test somewhere?

[SimenB]

Rebase this?

[jeysal]

@SimenB done. But we still need @scotthovestadt to verify that FB no longer has these weird hook definitions that would break because of this. Or would you do that when releasing an alpha an this has already landed anyway, instead of from this branch while it's open?

[SimenB]

Whatever's most convenient for @scotthovestadt 🙂

[jeysal]

GitHub not realizing I've pushed the rebase ... https://www.githubstatus.com "Degraded Performance"

[jeysal]

(And ofc would have to remove beforeEachQueue e2e test that fails on circus once we think that weird behavior no longer needs to be supported before merging ) ⚠️

[SimenB]

@scotthovestadt could you test this?

[SimenB]

@jeysal I've completely forgotten about this one. I still think it makes sense 😀 @cpojer will this break FB, or can we land it?

[cpojer]

No, this is a good change (y)

[cpojer]

👍

[github-actions]

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.