Skip to content

no-relative-import-in-test: also skip .d.mts/cts #724

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.

Sign up for GitHub

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

Jump to bottom
Merged
sandersn merged 1 commit into from
Aug 31, 2023
Merged

no-relative-import-in-test: also skip .d.mts/cts #724

sandersn merged 1 commit into from
Aug 31, 2023

Conversation

@sandersn
Copy link
Member

@sandersn sandersn commented Aug 31, 2023

I haven't figured out to get the test to pass for this, so I left it untested. Fixes the failure in fs-extra on DT.

@sandersn
no-relative-import-in-test: also skip .d.mts/cts
6ce2881 
I haven't figured out to get the test to pass for this, so I left it
untested.
@sandersn sandersn requested a review from andrewbranch as a code owner August 31, 2023 16:00
@sandersn
Copy link
Member Author

sandersn commented Aug 31, 2023

@JoshuaKGoldberg I could never figure out the tests for this rule, and I don't have time. I'm going to merge this, but someday I'd like to understand how ts-eslint tests work.

@sandersn sandersn merged commit 9f36d89 into microsoft:master Aug 31, 2023
@sandersn sandersn deleted the no-relative-import-in-test-skip-d.mts/cts branch August 31, 2023 16:07
@JoshuaKGoldberg
Copy link
Contributor

JoshuaKGoldberg commented Aug 31, 2023

cc @bradzacher - a good area for us to bolster docs on. Or maybe just good motivation for typescript-eslint/typescript-eslint#1891

@bradzacher
Copy link

bradzacher commented Aug 31, 2023

This one should be pretty simple to test.

Eg something like

valid: [
  {
    code: "some code that would normally error for the rule",
    filename: "foo.d.ts"
  },
  {
    code: "some code that would normally error for the rule",
    filename: "foo.d.cts"
  },
  {
    code: "some code that would normally error for the rule",
    filename: "foo.d.dts"
  },
]

You can use the filename prop to override the filename that is passed to ESLint and thus test this branch of logic.

Otherwise we have some docs on testing rules https://typescript-eslint.io/developers/custom-rules#testing

@sandersn
Copy link
Member Author

sandersn commented Sep 1, 2023

Adding the d.cts test produces this output.

The new valid test uses the same code as the invalid test, but should pass because it's in a d.cts file. However, both the new valid and existing invalid test now fail.

 FAIL  packages/eslint-plugin/test/no-relative-import-in-test.test.ts
  no-relative-import-in-test
    valid
      ✓ import ts from "does-not-exist"; (729 ms)
      ✓ import ts from "typescript"; (27 ms)
      ✓ import other from "./does-not-exit"; (26 ms)
      ✕ import abc from "./no-relative-import-in-test/abc.d.ts"; (43 ms)
    invalid
      ✕ import abc from "./no-relative-import-in-test/abc.d.ts"; (3 ms)

  ● no-relative-import-in-test › valid › import abc from "./no-relative-import-in-test/abc.d.ts";

    assert(received)

    Expected value to be equal to:
      true
    Received:
      false
    
    Message:
      A fatal parsing error occurred: Parsing error: ESLint was configured to run on `<tsconfigRootDir>/file.d.cts` using `parserOptions.project`: <tsconfigRootDir>/tsconfig.no-relative-import-in-test.json
    However, that TSConfig does not include this file. Either:
    - Change ESLint's list of included files to not include this file
    - Change that TSConfig to include this file
    - Create a new TSConfig that includes this file and include it in your parserOptions.project
    See the typescript-eslint docs for more info: https://typescript-eslint.io/linting/troubleshooting#i-get-errors-telling-me-eslint-was-configured-to-run--however-that-tsconfig-does-not--none-of-those-tsconfigs-include-this-file

      at runRuleForItem (node_modules/eslint/lib/rule-tester/rule-tester.js:613:13)
      at testValidTemplate (node_modules/eslint/lib/rule-tester/rule-tester.js:659:28)
      at Object.<anonymous> (node_modules/eslint/lib/rule-tester/rule-tester.js:947:29)

  ● no-relative-import-in-test › invalid › import abc from "./no-relative-import-in-test/abc.d.ts";

    assert(received)

    Expected value to be equal to:
      true
    Received:
      false
    
    Message:
      A fatal parsing error occurred: Parsing error: ESLint was configured to run on `<tsconfigRootDir>/file.ts` using `parserOptions.project`: <tsconfigRootDir>/tsconfig.no-relative-import-in-test.json
    However, that TSConfig does not include this file. Either:
    - Change ESLint's list of included files to not include this file
    - Change that TSConfig to include this file
    - Create a new TSConfig that includes this file and include it in your parserOptions.project
    See the typescript-eslint docs for more info: https://typescript-eslint.io/linting/troubleshooting#i-get-errors-telling-me-eslint-was-configured-to-run--however-that-tsconfig-does-not--none-of-those-tsconfigs-include-this-file

      at runRuleForItem (node_modules/eslint/lib/rule-tester/rule-tester.js:613:13)
      at testInvalidTemplate (node_modules/eslint/lib/rule-tester/rule-tester.js:709:28)
      at Object.<anonymous> (node_modules/eslint/lib/rule-tester/rule-tester.js:958:29)

@bradzacher
Copy link

bradzacher commented Sep 1, 2023

Oh sorry. I forgot that if the file doesn't exist then it's a parser error.

Wherever your test setup's tsconfig is - you'll need to create both files (they can be empty) and ensure the tsconfig includes them.

Sadly without a vfs things need to exist on disk for the type checker.

@sandersn sandersn mentioned this pull request Sep 8, 2023
Merged
sandersn added a commit to DefinitelyTyped/DefinitelyTyped that referenced this pull request Sep 8, 2023
@sandersn
Disable improved eslint rules
1351a39 
Found by microsoft/DefinitelyTyped-tools#723
         microsoft/DefinitelyTyped-tools#724
         microsoft/DefinitelyTyped-tools#726
sandersn added a commit to DefinitelyTyped/DefinitelyTyped that referenced this pull request Sep 11, 2023
@sandersn
Disable improved eslint rules (#66645)
e34c476 
* Disable improved eslint rules

Found by microsoft/DefinitelyTyped-tools#723
         microsoft/DefinitelyTyped-tools#724
         microsoft/DefinitelyTyped-tools#726

* add missed package
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andrewbranch andrewbranch Awaiting requested review from andrewbranch

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sandersn @JoshuaKGoldberg @bradzacher