Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

chore(eslint-plugin): consistently useit in tests#10680

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletionspackages/eslint-plugin/tests/areOptionsValid.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -17,16 +17,16 @@ const exampleRule = createRule<['value-a' | 'value-b'], never>({
name: 'my-example-rule',
});

test('returns true for valid options', () => {
it('returns true for valid options', () => {
expect(areOptionsValid(exampleRule, ['value-a'])).toBe(true);
});

describe('returns false for invalid options', () => {
test('bad enum value', () => {
it('bad enum value', () => {

Choose a reason for hiding this comment

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

🤔 I actually liketest() here overit(). The test name doesn't grammatically read out as a full sentence. I've personally kind of gravitated towards:

  • it(): for most tests, if there's at all a way to phrase it as"it X when Y" or something like that
  • test(): as a fallback if there's no logic, just a descriptor - like"test X"

Copy link
ContributorAuthor

@43081j43081jJan 19, 2025•
edited
Loading

Choose a reason for hiding this comment

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

I don't really care which one we use as long as it is one, and not two

If you can get the others to agree, I'm happy to update

I don't think you should end up with both. What you say makes sense but, if anything, suggests you would prefer havingtest throughout

Choose a reason for hiding this comment

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

Oh don't get me wrong, I thinkit() is the right choice almost all of the time. Just mypersonal nitpicky preference is to usetest() inspecific situations. 😄

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

To be honest, my preference is actually suite/test/assert. But the most consistent thing we can do right now isit it seems

As a middle ground, id prefer to later move from the now-consistentit totest if anything

expect(areOptionsValid(exampleRule, ['value-c'])).toBe(false);
});

test('bad type', () => {
it('bad type', () => {
expect(areOptionsValid(exampleRule, [true])).toBe(false);
});
});
18 changes: 9 additions & 9 deletionspackages/eslint-plugin/tests/docs.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -221,7 +221,7 @@ describe('Validating rule docs', () => {
const filePath = path.join(docsRoot, `${ruleName}.mdx`);
const { fullText, tokens } = parseMarkdownFile(filePath);

test(`${ruleName}.mdx must start with frontmatter description`, () => {
it(`${ruleName}.mdx must start with frontmatter description`, () => {
expect(tokens[0]).toMatchObject({
raw: '---\n',
type: 'hr',
Expand All@@ -235,7 +235,7 @@ describe('Validating rule docs', () => {
});
});

test(`${ruleName}.mdx must next have a blockquote directing to website`, () => {
it(`${ruleName}.mdx must next have a blockquote directing to website`, () => {
expect(tokens[4]).toMatchObject({
text: [
`🛑 This file is source code, not the primary documentation location! 🛑`,
Expand All@@ -247,7 +247,7 @@ describe('Validating rule docs', () => {
});
});

test(`headings must be title-cased`, () => {
it(`headings must be title-cased`, () => {
// Get all H2 headings objects as the other levels are variable by design.
const headings = tokens.filter(tokenIsH2);

Expand All@@ -269,7 +269,7 @@ describe('Validating rule docs', () => {
...requiredHeadings,
]);

test('important headings must be h2s', () => {
it('important headings must be h2s', () => {
for (const heading of headings) {
if (importantHeadings.has(heading.raw.replaceAll('#', '').trim())) {
expect(heading.depth).toBe(2);
Expand All@@ -278,7 +278,7 @@ describe('Validating rule docs', () => {
});

if (!rules[ruleName as keyof typeof rules].meta.docs?.extendsBaseRule) {
test('must include required headings', () => {
it('must include required headings', () => {
const headingTexts = new Set(
tokens.filter(tokenIsH2).map(token => token.text),
);
Expand DownExpand Up@@ -314,7 +314,7 @@ describe('Validating rule docs', () => {
for (const property of Object.keys(
schemaItem.properties as object,
)) {
test(property, () => {
it(property, () => {
const correspondingHeadingIndex =
headingsAfterOptions.findIndex(heading =>
heading.text.includes(`\`${property}\``),
Expand DownExpand Up@@ -359,7 +359,7 @@ describe('Validating rule docs', () => {
});
}

test('must include only valid code samples', () => {
it('must include only valid code samples', () => {
for (const token of tokens) {
if (token.type !== 'code') {
continue;
Expand All@@ -385,7 +385,7 @@ describe('Validating rule docs', () => {
}
});

test('code examples ESLint output', () => {
it('code examples ESLint output', () => {
// TypeScript can't infer type arguments unless we provide them explicitly
linter.defineRule<
keyof (typeof rule)['meta']['messages'],
Expand DownExpand Up@@ -528,7 +528,7 @@ ${token.value}`,
}
});

test('There should be no obsolete ESLint output snapshots', () => {
it('There should be no obsolete ESLint output snapshots', () => {
const files = fs.readdirSync(eslintOutputSnapshotFolder);
const names = new Set(Object.keys(rules).map(k => `${k}.shot`));

Expand Down
4 changes: 2 additions & 2 deletionspackages/eslint-plugin/tests/rules/array-type.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2232,14 +2232,14 @@ type BrokenArray = {

describe('schema validation', () => {
// https://github.com/typescript-eslint/typescript-eslint/issues/6852
test("array-type does not accept 'simple-array' option", () => {
it("array-type does not accept 'simple-array' option", () => {
if (areOptionsValid(rule, [{ default: 'simple-array' }])) {
throw new Error(`Options succeeded validation for bad options`);
}
});

// https://github.com/typescript-eslint/typescript-eslint/issues/6892
test('array-type does not accept non object option', () => {
it('array-type does not accept non object option', () => {
if (areOptionsValid(rule, ['array'])) {
throw new Error(`Options succeeded validation for bad options`);
}
Expand Down
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -964,7 +964,7 @@ describe('fixer should not change runtime value', () => {
continue;
}

test(code, () => {
it(code, () => {
expect(eval(code)).toEqual(
eval(Array.isArray(output) ? output.at(-1)! : output),
);
Expand Down
6 changes: 3 additions & 3 deletionspackages/eslint-plugin/tests/schemas.test.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -104,7 +104,7 @@ describe('Rule schemas should be convertible to TS types for documentation purpo
}
});

test('There should be no old snapshots for rules that have been deleted', () => {
it('There should be no old snapshots for rules that have been deleted', () => {
const files = fs.readdirSync(snapshotFolder);
const names = new Set(
Object.keys(rules)
Expand DownExpand Up@@ -191,7 +191,7 @@ describe('Rule schemas should validate options correctly', () => {
};

for (const [ruleName, rule] of Object.entries(rules)) {
test(`${ruleName} must accept valid options`, () => {
it(`${ruleName} must accept valid options`, () => {
if (
!areOptionsValid(
rule,
Expand All@@ -202,7 +202,7 @@ describe('Rule schemas should validate options correctly', () => {
}
});

test(`${ruleName} rejects arbitrary options`, () => {
it(`${ruleName} rejects arbitrary options`, () => {
if (areOptionsValid(rule, [{ 'arbitrary-schemas.test.ts': true }])) {
throw new Error(`Options succeeded validation for arbitrary options`);
}
Expand Down
Loading
[8]ページ先頭
©2009-2025 Movatter.jp