-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
test(plugin-typescript): [RFC] add a snapshot-based rule testing system #77
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
Conversation
What do you think about the style? |
This comment has been minimized.
This comment has been minimized.
(this reduces output in CI by about half)
I've provided some early feedback in spectrum, let's chat about it there :) |
this snapshot-based system looks nice. it seems like ESLint also supports endLine and endColumn information for errors. can that be surfaced in the snapshots somehow (we do it in TSLint)? |
I love this idea (you know I wanted to do something like this in the past). Some feedback I have (note this is based off not running the code....):
|
Perhaps we should do it like TSLint does? Have a directory for each rule, with subdirectories for each test. Each could have a |
Personally I don't find this worth the effort... If the test snapshot file uses a clear enough syntax (TSLint
+1, this change shouldn't be too hard to incorporate into this PR, though, I think it's just a matter of making a new (let's call it "testSnap") formatter which generates more reviewable snapshots?
+1 for this, I think using subdirectories for each different rule option configuration (rather than each individual test case) is a good middle ground. Instead of |
@adidahiya if you've got a format like that, that certainly helps a lot in terms of clarity! Having separate valid/invalid files might be good because it's more explicit in cases where there's multiple lines. |
Here’s the data ESLint gives us: {
fatal: false,
ruleId: "semi",
severity: 2,
line: 1,
column: 23,
message: "Expected a semicolon.",
fix: {
range: [1, 15],
text: ";"
}
} |
Huh, is that all? Well then we only want line, column and message. |
one other question I've got - are we thinking of supporting an api for this to generate tests? For example, for |
Ah, they do have
Maybe a
I wasn’t planning on it — if you want to do something fancy, you can use the |
I wish to see the fixed code beside the original code. How about a comment on the next line of the original code? foo.indexOf("bar") === 0
/*fixedTo:
foo.startsWith("bar")
*/ |
We can write options in code: /*eslint arbitrary-rule-name: [error, { option: true }] */
some code So I think it's enough we can separate tests to multiple files.
|
I'm going to close this for housekeeping purposes. |
No description provided.