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: migrate to createRuleTestCaseFunction #184

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft
5 changes: 5 additions & 0 deletions .changeset/fluffy-dolls-pump.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-import-x": patch
---

fix(first): add missing value in options type
Copy link
Collaborator

@SukkaW SukkaW Nov 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fix(first): add missing value in options type
fix(first): improves the type declaration of the rule `first`'s option

2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
eslint:
- '8.56'
- '8'
- '9'
Copy link
Author

@marcalexiei marcalexiei Nov 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yesterday the task was working

Today is not

It seems an issue with eslint: eslint/eslint#19134.
Relative eslint-plugin-unicorn issue: sindresorhus/eslint-plugin-unicorn#2496

I assume that the following steps in CI workflows ends up installing the last version of eslint:

- name: Install ESLint ${{ matrix.eslint }}
run: |
yarn add -D --ignore-engines eslint@${{ matrix.eslint }}
- name: Install Dependencies
run: yarn --ignore-engines
env:
SKIP_YARN_COREPACK_CHECK: 1

eslint:
- '8.56'
- '8'
- '9'

Replacing 9 with 9.14 makes CI green again.


I just want to report that on the failing CI the eslint version printed in the error is 9.8.0 which seems wrong since the error is present on 9.15.0.

Copy link
Collaborator

@SukkaW SukkaW Nov 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the issue comes from the typescript-eslint now. Let's wait and see if bumping typescript-eslint would be possible.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, and the same issue is affecting eslint-plugin-unicorn: sindresorhus/eslint-plugin-unicorn#2496

- '9.14'

include:
- executeLint: true
Expand Down
4 changes: 3 additions & 1 deletion src/rules/first.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ function isPossibleDirective(node: TSESTree.ProgramStatement) {
)
}

type Options = 'absolute-first' | 'disable-absolute-first'

type MessageId = 'absolute' | 'order'

export = createRule<['absolute-first'?], MessageId>({
export = createRule<[Options?], MessageId>({
name: 'first',
meta: {
type: 'suggestion',
Expand Down
4 changes: 3 additions & 1 deletion src/rules/group-exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ function accessorChain(node: TSESTree.MemberExpression) {
return chain
}

export = createRule<[], 'ExportNamedDeclaration' | 'AssignmentExpression'>({
type MessageId = 'ExportNamedDeclaration' | 'AssignmentExpression'

export = createRule<[], MessageId>({
name: 'group-exports',
meta: {
type: 'suggestion',
Expand Down
11 changes: 7 additions & 4 deletions test/rules/consistent-type-specifier-style.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import { RuleTester as TSESLintRuleTester } from '@typescript-eslint/rule-tester'
import { TSESTree } from '@typescript-eslint/utils'

import { parsers, test } from '../utils'
import { parsers, createRuleTestCaseFunction } from '../utils'
import type { RunTests } from '../utils'

import rule from 'eslint-plugin-import-x/rules/consistent-type-specifier-style'

const COMMON_TESTS = {
const test = createRuleTestCaseFunction<typeof rule>()

const COMMON_TESTS: RunTests<typeof rule> = {
valid: [
//
// prefer-top-level
Expand Down Expand Up @@ -248,7 +251,7 @@ import {
],
} as const

const TS_ONLY = {
const TS_ONLY: RunTests<typeof rule> = {
valid: [
//
// always valid
Expand All @@ -258,7 +261,7 @@ const TS_ONLY = {
invalid: [],
}

const FLOW_ONLY = {
const FLOW_ONLY: RunTests<typeof rule> = {
valid: [
//
// prefer-top-level
Expand Down
97 changes: 81 additions & 16 deletions test/rules/default.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,20 @@ import path from 'node:path'

import { RuleTester as TSESLintRuleTester } from '@typescript-eslint/rule-tester'

import { test, SYNTAX_CASES, parsers } from '../utils'
import {
createRuleTestCaseFunction,
SYNTAX_VALID_CASES,
parsers,
} from '../utils'
import type { RunTests } from '../utils'

import rule from 'eslint-plugin-import-x/rules/default'
import { CASE_SENSITIVE_FS } from 'eslint-plugin-import-x/utils'

const ruleTester = new TSESLintRuleTester()

const test = createRuleTestCaseFunction<typeof rule>()

ruleTester.run('default', rule, {
valid: [
test({
Expand Down Expand Up @@ -123,7 +130,7 @@ ruleTester.run('default', rule, {
},
}),

...SYNTAX_CASES,
...(SYNTAX_VALID_CASES as RunTests<typeof rule>['valid']),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to create a minimal type of ValidTestCase that is rule-agnostic? And we can type SYNTAX_VALID_CASES with that?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is already done on utils:

export const SYNTAX_VALID_CASES: TSESLintRunTests<string, unknown[]>['valid']

but this is will be never assignable to something that is rule specific like in these scenario where the SYNTAX_VALID_CASES is provided directly to a RuleTester instance.
Maybe I can create a createSyntaxValidCases function that infers the type and avoid to performs an explicit casting but IMHO casting is fine:
we have a generic value that is assigned to something more specific so it can be safely casted to that specific value.

],

invalid: [
Expand All @@ -138,9 +145,10 @@ ruleTester.run('default', rule, {
code: 'import baz from "./named-exports";',
errors: [
{
message:
'No default export found in imported module "./named-exports".',
type: 'ImportDefaultSpecifier',
messageId: 'noDefaultExport',
data: {
module: './named-exports',
},
},
],
}),
Expand All @@ -149,31 +157,64 @@ ruleTester.run('default', rule, {
test({
code: 'export baz from "./named-exports"',
languageOptions: { parser: require(parsers.BABEL) },
errors: ['No default export found in imported module "./named-exports".'],
errors: [
{
messageId: 'noDefaultExport',
data: {
module: './named-exports',
},
},
],
}),
test({
code: 'export baz, { bar } from "./named-exports"',
languageOptions: { parser: require(parsers.BABEL) },
errors: ['No default export found in imported module "./named-exports".'],
errors: [
{
messageId: 'noDefaultExport',
data: {
module: './named-exports',
},
},
],
}),
test({
code: 'export baz, * as names from "./named-exports"',
languageOptions: { parser: require(parsers.BABEL) },
errors: ['No default export found in imported module "./named-exports".'],
errors: [
{
messageId: 'noDefaultExport',
data: {
module: './named-exports',
},
},
],
}),
// exports default from a module with no default
test({
code: 'import twofer from "./broken-trampoline"',
languageOptions: { parser: require(parsers.BABEL) },
errors: [
'No default export found in imported module "./broken-trampoline".',
{
messageId: 'noDefaultExport',
data: {
module: './broken-trampoline',
},
},
],
}),

// #328: * exports do not include default
test({
code: 'import barDefault from "./re-export"',
errors: ['No default export found in imported module "./re-export".'],
errors: [
{
messageId: 'noDefaultExport',
data: {
module: './re-export',
},
},
],
}),
],
})
Expand All @@ -190,7 +231,12 @@ if (!CASE_SENSITIVE_FS) {
test({
code: 'import bar from "./Named-Exports"',
errors: [
'No default export found in imported module "./Named-Exports".',
{
messageId: 'noDefaultExport',
data: {
module: './Named-Exports',
},
},
],
}),
],
Expand Down Expand Up @@ -321,7 +367,14 @@ describe('TypeScript', () => {
'import-x/parsers': { [parsers.TS]: ['.ts'] },
'import-x/resolver': { 'eslint-import-resolver-typescript': true },
},
errors: ['No default export found in imported module "./typescript".'],
errors: [
{
messageId: 'noDefaultExport',
data: {
module: './typescript',
},
},
],
}),
test({
code: `import React from "./typescript-export-assign-default-namespace"`,
Expand All @@ -331,7 +384,12 @@ describe('TypeScript', () => {
'import-x/resolver': { 'eslint-import-resolver-typescript': true },
},
errors: [
'No default export found in imported module "./typescript-export-assign-default-namespace".',
{
messageId: 'noDefaultExport',
data: {
module: './typescript-export-assign-default-namespace',
},
},
],
}),
test({
Expand All @@ -342,7 +400,12 @@ describe('TypeScript', () => {
'import-x/resolver': { 'eslint-import-resolver-typescript': true },
},
errors: [
'No default export found in imported module "./typescript-export-as-default-namespace".',
{
messageId: 'noDefaultExport',
data: {
module: './typescript-export-as-default-namespace',
},
},
],
}),
test({
Expand All @@ -362,8 +425,10 @@ describe('TypeScript', () => {
},
errors: [
{
message:
'No default export found in imported module "./typescript-export-as-default-namespace".',
messageId: 'noDefaultExport',
data: {
module: './typescript-export-as-default-namespace',
},
line: 1,
column: 8,
endLine: 1,
Expand Down
15 changes: 9 additions & 6 deletions test/rules/dynamic-import-chunkname.spec.ts
Original file line number Diff line number Diff line change
@@ -1,39 +1,42 @@
import { RuleTester as TSESLintRuleTester } from '@typescript-eslint/rule-tester'
import { TSESTree } from '@typescript-eslint/utils'

import { SYNTAX_CASES, parsers } from '../utils'
import { SYNTAX_VALID_CASES, parsers } from '../utils'
import type { GetRuleModuleOptions } from '../utils'

import rule from 'eslint-plugin-import-x/rules/dynamic-import-chunkname'

const ruleTester = new TSESLintRuleTester()

type RuleOptions = GetRuleModuleOptions<typeof rule>

const pickyCommentFormat = '[a-zA-Z-_/.]+'

const options = [
{
importFunctions: ['dynamicImport'],
},
] as const
] as const satisfies RuleOptions

const pickyCommentOptions = [
{
importFunctions: ['dynamicImport'],
webpackChunknameFormat: pickyCommentFormat,
},
] as const
] as const satisfies RuleOptions

const allowEmptyOptions = [
{
importFunctions: ['dynamicImport'],
allowEmpty: true,
},
] as const
] as const satisfies RuleOptions

const multipleImportFunctionOptions = [
{
importFunctions: ['dynamicImport', 'definitelyNotStaticImport'],
},
] as const
] as const satisfies RuleOptions

const babelParser = require(parsers.BABEL)

Expand Down Expand Up @@ -425,7 +428,7 @@ ruleTester.run('dynamic-import-chunkname', rule, {
options,
languageOptions: { parser: babelParser },
},
...SYNTAX_CASES,
...SYNTAX_VALID_CASES,
],

invalid: [
Expand Down
Loading
Loading