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

test(eslint-plugin): migrate validation tools to jest test cases #1403

Merged
merged 6 commits into from Jan 7, 2020

Conversation

@armano2
Copy link
Member

armano2 commented Jan 5, 2020

This is follow up PR that changes way how we do test configs and documentations.

There is few positives and negatives with changing it to tests:

  • Custom tools are more descriptive and can/show custom messages like "run x command"
  • Tests uses common framework to validate if configurations/documents are valid, it means that they can be simpler, because we can utilize jest functions (like comparing objects), that allows us to reduce complexity.

For example new configs validation is going to look like this

all.json config › contains all @typescript-eslint/eslint-plugin rule modules, except the deprecated ones

expect(received).toEqual(expected) // deep equality
- Expected
+ Received

@@ -2,10 +2,11 @@
   "@typescript-eslint/adjacent-overload-signatures": "error",
   "@typescript-eslint/array-type": "error",
   "@typescript-eslint/await-thenable": "error",
   "@typescript-eslint/ban-ts-ignore": "error",
   "@typescript-eslint/ban-types": "error",
+  "@typescript-eslint/brace-style": "error",
   "@typescript-eslint/camelcase": "error",
   "@typescript-eslint/class-name-casing": "error",
   "@typescript-eslint/consistent-type-assertions": "error",
   "@typescript-eslint/consistent-type-definitions": "error",
   "@typescript-eslint/explicit-function-return-type": "error",

TODO:

  • update test tiles

#1396 (comment)

@typescript-eslint

This comment was marked as resolved.

Copy link

typescript-eslint bot commented Jan 5, 2020

Thanks for the PR, @armano2!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!


🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

Copy link
Member

bradzacher left a comment

few nits on the test wording.
might as well be more explicit whilst we're at it.

otherwise, LGTM - thanks for doing this

packages/eslint-plugin/tests/configs.test.ts Outdated Show resolved Hide resolved
packages/eslint-plugin/tests/configs.test.ts Outdated Show resolved Hide resolved
packages/eslint-plugin/tests/configs.test.ts Outdated Show resolved Hide resolved
// find the table
const rulesTable = readme.find(
token => token.type === 'table',
) as marked.Tokens.Table;

This comment has been minimized.

Copy link
@bradzacher

bradzacher Jan 5, 2020

Member

Nit - I probably should have correctly typed this

Suggested change
) as marked.Tokens.Table;
) as marked.Tokens.Table | undefined;

This comment has been minimized.

Copy link
@armano2

armano2 Jan 7, 2020

Author Member

that's correct, i just copy/paste it from tools :>

This comment has been minimized.

Copy link
@armano2

armano2 Jan 7, 2020

Author Member

this actually should be:

Suggested change
) as marked.Tokens.Table;
const rulesTable = readme.find(
(token): token is marked.Tokens.Table => token.type === 'table',
);
packages/eslint-plugin/tests/docs.test.ts Outdated Show resolved Hide resolved
packages/eslint-plugin/tests/docs.test.ts Outdated Show resolved Hide resolved
packages/eslint-plugin/tests/docs.test.ts Outdated Show resolved Hide resolved
packages/eslint-plugin/tests/docs.test.ts Outdated Show resolved Hide resolved
packages/eslint-plugin/tests/docs.test.ts Outdated Show resolved Hide resolved
packages/eslint-plugin/tests/docs.test.ts Outdated Show resolved Hide resolved
@bradzacher bradzacher added the tests label Jan 5, 2020
armano2 added 4 commits Jan 7, 2020
…t-eslint into tools-to-tests
@armano2 armano2 marked this pull request as ready for review Jan 7, 2020
Copy link
Member

bradzacher left a comment

LGTM - thanks!

@bradzacher bradzacher merged commit 40d9127 into typescript-eslint:master Jan 7, 2020
7 checks passed
7 checks passed
Semantic Pull Request ready to be squashed
Details
codecov/patch Coverage not affected when comparing f7ad716...872a318
Details
codecov/project 94.58% remains the same compared to f7ad716
Details
typescript-eslint.typescript-eslint Build #20200107.9 succeeded
Details
typescript-eslint.typescript-eslint (Primary code validation and tests) Primary code validation and tests succeeded
Details
typescript-eslint.typescript-eslint (Run unit tests on other Node.js versions node_10_x) Run unit tests on other Node.js versions node_10_x succeeded
Details
typescript-eslint.typescript-eslint (Run unit tests on other Node.js versions node_8_x) Run unit tests on other Node.js versions node_8_x succeeded
Details
@armano2 armano2 deleted the armano2:tools-to-tests branch Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.