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

Bug: V9 can't test non-exists file in RuleTester #17962

Closed
1 task
fisker opened this issue Jan 6, 2024 · 5 comments · Fixed by #17989
Closed
1 task

Bug: V9 can't test non-exists file in RuleTester #17962

fisker opened this issue Jan 6, 2024 · 5 comments · Fixed by #17989
Assignees
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly repro:yes Issues with a reproducible example

Comments

@fisker
Copy link
Contributor

fisker commented Jan 6, 2024

Environment

Node version: v18.18.0
npm version: v9.4.2
Local ESLint version: v9.0.0-alpha.0 (Currently used)
Global ESLint version: Not found
Operating System: linux 5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/122.0.0.0 Safari/537.36

What parser are you using?

Default (Espree)

What did you do?

Create a file to run RuleTester

import { RuleTester } from 'eslint';

const tester = new RuleTester();

const rule = {
  create: (context) => ({
    Program(node) {
      context.report({ node, message: 'It works' });
    },
  }),
};

tester.run('rule-id', rule, {
  valid: [],
  invalid: [
    {
      code: '_',
      filename: '/a-none-exits-file',
      errors: [{ message: 'It works' }],
    },
  ],
});

What did you expect to happen?

Should pass.

What actually happened?

Error: Error rule name should be the same as the name of the rule being tested (false == true)
    at _0x2e26db._evaluate (https://stackblitzstartersdvpau5-bahv.w-credentialless.staticblitz.com/blitz.a2aabdd9.js:352:376700)
    at async ModuleJob.run (https://stackblitzstartersdvpau5-bahv.w-credentialless.staticblitz.com/blitz.a2aabdd9.js:181:2372) {
  generatedMessage: false,
  code: 'ERR_ASSERTION',
  actual: false,
  expected: true,
  operator: '=='
}

Link to Minimal Reproducible Example

https://stackblitz.com/edit/stackblitz-starters-dvpau5?description=Starter%20project%20for%20Node.js,%20a%20JavaScript%20runtime%20built%20on%20Chrome%27s%20V8%20JavaScript%20engine&file=index.js,package.json&title=node.new%20Starter

Participation

  • I am willing to submit a pull request for this issue.

Additional comments

No response

@fisker fisker added bug ESLint is working incorrectly repro:needed This issue should include a reproducible example labels Jan 6, 2024
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Jan 6, 2024
@fisker fisker changed the title Bug: V9 can't test non-exits file in RuleTester Bug: V9 can't test non-exists file in RuleTester Jan 6, 2024
@mdjermanovic
Copy link
Member

filename: '/a-none-exits-file',

The problem is that absolute paths are not supported here (unless it happens that the path is in or under the current working directory). It would work with filename: 'a-none-exits-file' regardless of whether the file exists.

Do you need absolute paths in your test cases?

@mdjermanovic mdjermanovic moved this from Needs Triage to Triaging in Triage Jan 6, 2024
@mdjermanovic mdjermanovic added repro:yes Issues with a reproducible example and removed repro:needed This issue should include a reproducible example labels Jan 6, 2024
@fisker
Copy link
Contributor Author

fisker commented Jan 7, 2024

Shouldn't RuleTester allow to test any case?

@mdjermanovic
Copy link
Member

I think that absolute paths should be supported because rules are getting absolute paths in production so testing with absolute paths seems more realistic. Perhaps we could pass path.dirname(item.filename) (or path.parse(item.filename).root?) as the basePath when creating a config array instance for the test.

function runRuleForItem(item) {
const configs = new FlatConfigArray(testerConfig, { baseConfig });

@nzakas what do you think?

@mdjermanovic mdjermanovic moved this from Triaging to Feedback Needed in Triage Jan 8, 2024
@nzakas
Copy link
Member

nzakas commented Jan 9, 2024

I think that absolute paths should be supported because rules are getting absolute paths in production so testing with absolute paths seems more realistic. Perhaps we could pass path.dirname(item.filename) (or path.parse(item.filename).root?) as the basePath when creating a config array instance for the test.

function runRuleForItem(item) {
const configs = new FlatConfigArray(testerConfig, { baseConfig });

@nzakas what do you think?

I think that makes sense. We should be able to support absolute paths in tests. 👍

@nzakas nzakas added the accepted There is consensus among the team that this change meets the criteria for inclusion label Jan 9, 2024
@nzakas nzakas moved this from Feedback Needed to Ready to Implement in Triage Jan 9, 2024
@snitin315 snitin315 self-assigned this Jan 10, 2024
@snitin315
Copy link
Contributor

snitin315 commented Jan 10, 2024

That makes sense, I'll work on this.

@github-project-automation github-project-automation bot moved this from Ready to Implement to Complete in Triage Jan 16, 2024
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Jul 15, 2024
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Jul 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly repro:yes Issues with a reproducible example
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants