Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allowed describe and it/test to take in functions again #12484

Merged
merged 18 commits into from Feb 24, 2022

Conversation

JoshuaKGoldberg
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg commented Feb 24, 2022

Summary

Partially replicates DefinitelyTyped/DefinitelyTyped#23102 by allowing describe to take in a number or function.

Fixes #12483.

Test plan

I can verify TypeScript thinks these things are valid:

var describe: jest.Describe (name: string | number | Function | jest.FunctionLike, fn: jest.EmptyFunction) => void

I couldn't figure out where the right place to test this at runtime is, though. Could someone help please? 馃檪 Actually, there are a bunch of compiler issues - I'll work on those.

@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as draft Feb 24, 2022
@JoshuaKGoldberg JoshuaKGoldberg changed the title Allowed describe and test to take in functions again Allowed describe to take in functions again Feb 24, 2022
@SimenB
Copy link
Collaborator

SimenB commented Feb 24, 2022

Nice! CI seems happy 馃檪

Can you add type tests as well? https://github.com/facebook/jest/blob/c65bb4521597fb1ad65dacd778a26e04684b2872/packages/jest-types/__typetests__/globals.test.ts

I couldn't figure out where the right place to test this at runtime is, though.

Runtime support is already test by the test you added in #5154, no?

test('function as descriptor', () => {
const filename = 'functionAsDescriptor.test.js';
const content = `
function Foo() {}
describe(Foo, () => {
it('it', () => {});
});
`;
writeFiles(TEST_DIR, {[filename]: content});
const {stderr, exitCode} = runJest(DIR);
const {summary, rest} = extractSummary(stderr);
expect(rest).toMatchSnapshot();
expect(summary).toMatchSnapshot();
expect(exitCode).toBe(0);
});

@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as ready for review Feb 24, 2022
@JoshuaKGoldberg JoshuaKGoldberg requested a review from SimenB Feb 24, 2022
Copy link
Collaborator

@SimenB SimenB left a comment

changelog entry as well? 馃檪

packages/jest-util/src/convertDescriptorToString.ts Outdated Show resolved Hide resolved
packages/jest-circus/src/index.ts Outdated Show resolved Hide resolved
@JoshuaKGoldberg JoshuaKGoldberg changed the title Allowed describe to take in functions again Allowed describe and it/test to take in functions again Feb 24, 2022
SimenB
SimenB approved these changes Feb 24, 2022
Copy link
Collaborator

@SimenB SimenB left a comment

thanks!

@SimenB SimenB merged commit 6b2e901 into facebook:main Feb 24, 2022
1 of 7 checks passed
@JoshuaKGoldberg JoshuaKGoldberg deleted the block-test-name-function-like branch Feb 24, 2022
@JoshuaKGoldberg
Copy link
Contributor Author

JoshuaKGoldberg commented Feb 24, 2022

Thank you for the pointers @SimenB! 馃檶

@github-actions
Copy link

github-actions bot commented Apr 26, 2022

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Types no longer allow passing a named function to describe
4 participants