test(linter/plugins): replace `assert` with vitest's `expect` by lilnasy · Pull Request #16016 · oxc-project/oxc

lilnasy · 2025-11-23T23:21:34Z

Follow up to review comment in feat(linter/plugins): implement SourceCode#getFirstToken() #16002 (comment).
We were originally using assert because it was a direct analogue to eslint testing, made migrating their tests slightly easier.

Copilot

Pull request overview

This pull request migrates test assertions from Node.js's assert module to vitest's expect API in the linter plugin tests. While the intent is good, the conversion contains critical bugs that will cause test failures.

Key Changes:

Removed assert import and added expect to vitest imports
Converted assert.deepStrictEqual() calls to expect().toEqual()
Converted assert.strictEqual() calls to expect().toBe()
Attempted conversion of assert.fail() to expect.fail()

Critical Issues Found:

15 instances where expect() calls are missing the matcher function (e.g., .toBe()), with the expected value incorrectly passed as a second argument to expect() instead
1 instance of invalid API usage: expect.fail() does not exist in vitest

Comments suppressed due to low confidence (15)

apps/oxlint/test/tokens.test.ts:356

Missing .toBe() matcher on the expect() call. The second argument 'C' should be part of .toBe('C') instead of being a separate argument to expect().

This should be:

expect(
  getTokenAfter(VariableDeclaratorIdentifier, {
    includeComments: true,
    skip: 2,
  })!.value,
).toBe('C');

    expect(
      getTokenAfter(VariableDeclaratorIdentifier, {
        includeComments: true,
        skip: 2,
      })!.value,
      'C',
    );

apps/oxlint/test/tokens.test.ts:268

Missing .toBe() matcher on the expect() call. The second argument '=' should be part of .toBe('=') instead of being a separate argument to expect().

This should be:

expect(
  getTokenBefore(BinaryExpression, {
    includeComments: true,
    skip: 1,
  })!.value,
).toBe('=');

    expect(
      getTokenBefore(BinaryExpression, {
        includeComments: true,
        skip: 1,
      })!.value,
      '=',
    );

apps/oxlint/test/tokens.test.ts:337

Missing .toBe() matcher on the expect() call. The second argument 'b' should be part of .toBe('b') instead of being a separate argument to expect().

This should be:

expect(
  getTokenAfter(VariableDeclaratorIdentifier, {
    skip: 1,
    filter: (t) => t.type === 'Identifier',
  })!.value,
).toBe('b');

    expect(
      getTokenAfter(VariableDeclaratorIdentifier, {
        skip: 1,
        filter: (t) => t.type === 'Identifier',
      })!.value,
      'b',
    );

apps/oxlint/test/tokens.test.ts:566

Missing .toBe() matcher on the expect() call. The second argument 'a' should be part of .toBe('a') instead of being a separate argument to expect().

This should be:

expect(
  getFirstToken(BinaryExpression, {
    filter: (t) => t.type === 'Identifier',
  })!.value,
).toBe('a');

    expect(
      getFirstToken(BinaryExpression, {
        filter: (t) => t.type === 'Identifier',
      })!.value,
      'a',
    );

apps/oxlint/test/tokens.test.ts:590

Missing .toBe() matcher on the expect() call. The second argument 'D' should be part of .toBe('D') instead of being a separate argument to expect().

This should be:

expect(
  getFirstToken(BinaryExpression, {
    includeComments: true,
    skip: 1,
  })!.value,
).toBe('D');

    expect(
      getFirstToken(BinaryExpression, {
        includeComments: true,
        skip: 1,
      })!.value,
      'D',
    );

apps/oxlint/test/tokens.test.ts:617

Missing .toBe() matcher on the expect() call. The second argument 'comment' should be part of .toBe('comment') instead of being a separate argument to expect().

This should be:

expect(
  getFirstToken(
    { range: [6, 23] } as Node,
    { includeComments: true },
  )!.value,
).toBe('comment');

    expect(
      getFirstToken(
        // TODO: this verbatim range should be replaced with `[ast.comments[0].range[0], ast.tokens[5].range[1]]`
        { range: [6, 23] } as Node,
        { includeComments: true },
      )!.value,
      'comment',
    );

apps/oxlint/test/tokens.test.ts:258

Missing .toBe() matcher on the expect() call. The second argument 'C' should be part of .toBe('C') instead of being a separate argument to expect().

This should be:

expect(
  getTokenBefore(BinaryExpression, {
    includeComments: true,
  })!.value,
).toBe('C');

    expect(
      getTokenBefore(BinaryExpression, {
        includeComments: true,
      })!.value,
      'C',
    );

apps/oxlint/test/tokens.test.ts:279

Missing .toBe() matcher on the expect() call. The second argument 'B' should be part of .toBe('B') instead of being a separate argument to expect().

This should be:

expect(
  getTokenBefore(BinaryExpression, {
    includeComments: true,
    skip: 1,
    filter: (t) => t.type.startsWith('Block'),
  })!.value,
).toBe('B');

    expect(
      getTokenBefore(BinaryExpression, {
        includeComments: true,
        skip: 1,
        filter: (t) => t.type.startsWith('Block'),
      })!.value,
      'B',
    );

apps/oxlint/test/tokens.test.ts:576

Missing .toBe() matcher on the expect() call. The second argument 'b' should be part of .toBe('b') instead of being a separate argument to expect().

This should be:

expect(
  getFirstToken(BinaryExpression, {
    skip: 1,
    filter: (t) => t.type === 'Identifier',
  })!.value,
).toBe('b');

    expect(
      getFirstToken(BinaryExpression, {
        skip: 1,
        filter: (t) => t.type === 'Identifier',
      })!.value,
      'b',
    );

apps/oxlint/test/tokens.test.ts:601

Missing .toBe() matcher on the expect() call. The second argument '*' should be part of .toBe('*') instead of being a separate argument to expect().

This should be:

expect(
  getFirstToken(BinaryExpression, {
    includeComments: true,
    skip: 1,
    filter: (t) => t.value !== 'a',
  })!.value,
).toBe('*');

    expect(
      getFirstToken(BinaryExpression, {
        includeComments: true,
        skip: 1,
        filter: (t) => t.value !== 'a',
      })!.value,
      '*',
    );

apps/oxlint/test/tokens.test.ts:632

Missing .toBe() matcher on the expect() call. The second argument 'c' should be part of .toBe('c') instead of being a separate argument to expect().

This should be:

expect(
  getFirstToken({
    range: [6, 23],
  } as Node)!.value,
).toBe('c');

    expect(
      getFirstToken({
        // TODO: this verbatim range should be replaced with `[ast.comments[0].range[0], ast.tokens[5].range[1]]`
        range: [6, 23],
      } as Node)!.value,
      'c',
    );

apps/oxlint/test/tokens.test.ts:249

Missing .toBe() matcher on the expect() call. The second argument 'var' should be part of .toBe('var') instead of being a separate argument to expect().

This should be:

expect(
  getTokenBefore(BinaryExpression, {
    skip: 1,
    filter: (t) => t.value !== '=',
  })!.value,
).toBe('var');

    expect(
      getTokenBefore(BinaryExpression, {
        skip: 1,
        filter: (t) => t.value !== '=',
      })!.value,
      'var',
    );

apps/oxlint/test/tokens.test.ts:327

Missing .toBe() matcher on the expect() call. The second argument 'a' should be part of .toBe('a') instead of being a separate argument to expect().

This should be:

expect(
  getTokenAfter(VariableDeclaratorIdentifier, {
    filter: (t) => t.type === 'Identifier',
  })!.value,
).toBe('a');

    expect(
      getTokenAfter(VariableDeclaratorIdentifier, {
        filter: (t) => t.type === 'Identifier',
      })!.value,
      'a',
    );

apps/oxlint/test/tokens.test.ts:346

Missing .toBe() matcher on the expect() call. The second argument 'B' should be part of .toBe('B') instead of being a separate argument to expect().

This should be:

expect(
  getTokenAfter(VariableDeclaratorIdentifier, {
    includeComments: true,
  })!.value,
).toBe('B');

    expect(
      getTokenAfter(VariableDeclaratorIdentifier, {
        includeComments: true,
      })!.value,
      'B',
    );

apps/oxlint/test/tokens.test.ts:367

Missing .toBe() matcher on the expect() call. The second argument 'D' should be part of .toBe('D') instead of being a separate argument to expect().

This should be:

expect(
  getTokenAfter(VariableDeclaratorIdentifier, {
    includeComments: true,
    skip: 2,
    filter: (t) => t.type.startsWith('Block'),
  })!.value,
).toBe('D');

    expect(
      getTokenAfter(VariableDeclaratorIdentifier, {
        includeComments: true,
        skip: 2,
        filter: (t) => t.type.startsWith('Block'),
      })!.value,
      'D',
    );

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

apps/oxlint/test/tokens.test.ts

overlookmotel

Thank you! I hope you used AI for this...

lilnasy · 2025-11-23T23:40:07Z

Yes, but oxlint is what saved me the time of correcting all of its mistakes! There were a few needles of in the haystack of missing .toBe calls.

overlookmotel · 2025-11-23T23:45:04Z

I've replaced .toBe(null) with .toBeNull(). That seems to be the "standard style" with Vitest.

Will merge as soon as CI passes.

…oject#16016) - Follow up to review comment in oxc-project#16002 (comment). - We were originally using `assert` because it was a direct analogue to `eslint` testing, made migrating their tests slightly easier. --------- Co-authored-by: overlookmotel <[email protected]>

Copilot AI review requested due to automatic review settings November 23, 2025 23:21

lilnasy requested a review from camc314 as a code owner November 23, 2025 23:21

github-actions bot added A-linter Area - Linter A-cli Area - CLI C-test Category - Testing. Code is missing test cases, or a PR is adding them labels Nov 23, 2025

Copilot started reviewing on behalf of lilnasy November 23, 2025 23:21 View session

Copilot finished reviewing on behalf of lilnasy November 23, 2025 23:24

Copilot AI reviewed Nov 23, 2025

View reviewed changes

apps/oxlint/test/tokens.test.ts Show resolved Hide resolved

lilnasy force-pushed the assert-expect branch from 2d78570 to 8fde60f Compare November 23, 2025 23:30

test(linter/plugins): replace assert with vitest's expect

b941034

lilnasy force-pushed the assert-expect branch from 8fde60f to b941034 Compare November 23, 2025 23:35

overlookmotel approved these changes Nov 23, 2025

View reviewed changes

.toBe(null) -> .toBeNull()

7b6e8b8

overlookmotel mentioned this pull request Nov 23, 2025

refactor(oxlint): use vitest expect rather than node:assert in test cases #15988

Closed

overlookmotel merged commit 0c48e28 into oxc-project:main Nov 23, 2025
17 checks passed

lilnasy deleted the assert-expect branch November 23, 2025 23:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

test(linter/plugins): replace `assert` with vitest's `expect`#16016

test(linter/plugins): replace `assert` with vitest's `expect`#16016
overlookmotel merged 2 commits intooxc-project:mainfrom
lilnasy:assert-expect

lilnasy commented Nov 23, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

overlookmotel left a comment

Uh oh!

lilnasy commented Nov 23, 2025

Uh oh!

overlookmotel commented Nov 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Comments

Conversation

lilnasy commented Nov 23, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

overlookmotel left a comment

Choose a reason for hiding this comment

Uh oh!

lilnasy commented Nov 23, 2025

Uh oh!

overlookmotel commented Nov 23, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants