Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

fix: improve support for it.each involving tagged template literals #701

Merged
merged 5 commits into from
Nov 12, 2020
Merged

fix: improve support for it.each involving tagged template literals #701

merged 5 commits into from
Nov 12, 2020

Conversation

k-yle
Copy link
Contributor

@k-yle k-yle commented Oct 28, 2020

it.each has an unusual syntax which means no-disabled-tests, no-test-prefixes, and consistent-test-it don't work for it.

This PR makes it.each work with those three rules

@SimenB SimenB requested a review from G-Rath October 28, 2020 10:15
G-Rath
G-Rath requested changes Nov 1, 2020
View reviewed changes
Copy link
Collaborator

@G-Rath G-Rath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, thanks for this - I'm not sure how we missed it for so long!

I've left a few nits, and asked to have the interface renamed and its export removed (since its not needed); otherwise I think this is good to go!

src/rules/__tests__/no-standalone-expect.test.ts Outdated Show resolved Hide resolved
src/rules/no-test-prefixes.ts
@@ -43,12 +49,14 @@ export default createRule({
function getPreferredNodeName(nodeName: string) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: a good refactor for this could be to do split('.') - then you could just splice the only/skip in as element 1, and return .join('.').

I'm fine with this how it is, so that doesn't have to be done - just thinking of the future :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'll leave this one as-is

src/rules/utils.ts Outdated Show resolved Hide resolved
src/rules/utils.ts Outdated Show resolved Hide resolved
src/rules/utils.ts Outdated Show resolved Hide resolved
@G-Rath G-Rath changed the title Fix bugs with it.each in many rules fix: improve support for it.each involving tagged template literals Nov 1, 2020
@k-yle k-yle requested a review from G-Rath November 1, 2020 03:34
@k-yle
Copy link
Contributor Author

k-yle commented Nov 11, 2020

hey @G-Rath, I've made the changes you requested - is there anything else I need to do?

G-Rath
G-Rath approved these changes Nov 12, 2020
View reviewed changes
Copy link
Collaborator

@G-Rath G-Rath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there anything else I need to do?

You need to prod me because I apparently forgot to approve and merge this 😅

@G-Rath G-Rath merged commit 2341814 into jest-community:master Nov 12, 2020
github-actions bot pushed a commit that referenced this pull request Nov 12, 2020
@semantic-release-bot
chore(release): 24.1.1 [skip ci]
ce08024 
## [24.1.1](v24.1.0...v24.1.1) (2020-11-12)

### Bug Fixes

* improve support for it.each involving tagged template literals ([#701](#701)) ([2341814](2341814))
@github-actions
Copy link

🎉 This PR is included in version 24.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@k-yle k-yle deleted the bugs-with-each branch November 12, 2020 08:22
@shobhitsharma
Copy link

@k-yle getting some error Return a Promise instead of relying on callback parameter jest/no-done-callback after this upgrade. Maybe more info for this change would help.

@k-yle
Copy link
Contributor Author

k-yle commented Nov 12, 2020

@shobhitsharma yep looks like this PR broke jest/no-done-callback for it.each 🙊 working on it right now

@k-yle k-yle mentioned this pull request Nov 12, 2020
Merged
@k-yle
Copy link
Contributor Author

k-yle commented Nov 12, 2020

fixed in #708

cc @G-Rath

SimenB added a commit that referenced this pull request Nov 12, 2020
@SimenB
Revert "fix: improve support for it.each involving tagged template li…
5cc1d1b 
…terals (#701)"

This reverts commit 2341814.
@SimenB SimenB mentioned this pull request Nov 12, 2020
Merged
@SimenB
Copy link
Member

SimenB commented Nov 12, 2020

@k-yle @G-Rath (is it a new zealand thing to go with that structure for usernames? 😀) I've revert this change in #713, but kept all the passing tests (plus an extra regression test for #710). A new go would be great 👍

@k-yle
Copy link
Contributor Author

k-yle commented Nov 13, 2020

@SimenB I won't have time until next Tuesday so if someone else wants to do it before then, go for it 👍

it may also be above my skill level to fix this edge case - I don't know how no-done-callback could work with that syntax

also, do we know how to reproduce #711?

@k-yle k-yle mentioned this pull request Nov 27, 2020
Merged
@Obi-Dann Obi-Dann mentioned this pull request Apr 10, 2021
Draft
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@SimenB SimenB SimenB left review comments

@G-Rath G-Rath G-Rath approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@k-yle @shobhitsharma @SimenB @G-Rath