testing-library/eslint-plugin-testing-library

False positive for await-async-utils

iantocristian opened this issue ยท 7 comments

Have you read the Troubleshooting section?

Yes

Plugin version

v5.10.1

ESLint version

v8.33.0

Node.js version

v18.12.1

package manager and version

npm 9.4.2

Operating system

macOS Ventura 13.2

Bug description

This appears to be a regression introduced with the latest patch release, v5.10.1. Not present in v5.10.0.

False positive rule await-async-utils triggered by inline async function definition on other variables and/or definitions.

Promise returned from x wrapper over async util must be handled but x is not a function.

Steps to reproduce

This code triggers a false positive on expectedValue:

import React from 'react';
import { render, act } from '@testing-library/react';

function wait(ms) {
  return new Promise((resolve) => {
    setTimeout(resolve, ms);
  });
}

const waitWithAct = async (timeout) => {
  await act(async () => await wait(timeout));
};

describe('Component', () => {
  const mock = jest.fn();

  it('test', async () => {
    let Component = () => {
      mock(1);
      return <div />;
    };
    render(<Component />);

    await waitWithAct(500);

    const expectedValue = 1;
    expect(mock).toHaveBeenCalledWith(expectedValue);
  });
});

Rewriting the inline async function fixes the false positive:

import React from 'react';
import { render, act } from '@testing-library/react';

function wait(ms) {
  return new Promise((resolve) => {
    setTimeout(resolve, ms);
  });
}

const waitWithAct = async (timeout) => {
  await act(async () => {
    await wait(timeout);
  });
};

describe('Component', () => {
  const mock = jest.fn();

  it('test', async () => {
    let Component = () => {
      mock(1);
      return <div />;
    };
    render(<Component />);

    await waitWithAct(500);

    const expectedValue = 1;
    expect(mock).toHaveBeenCalledWith(expectedValue);
  });
});

Another case of false positive, this results in false positives reported for mock and jest.fn:

import React from 'react';
import { render, act } from '@testing-library/react';

function wait(ms) {
  return new Promise((resolve) => {
    setTimeout(resolve, ms);
  });
}

const waitWithAct = async (timeout) => {
  const fn = async () => await wait(timeout);
  await act(fn);
};

describe('Component', () => {
  const mock = jest.fn();

  it('test', async () => {
    let Component = () => {
      mock(1);
      return <div />;
    };
    render(<Component />);

    await waitWithAct(500);

    const expectedValue = 1;
    expect(mock).toHaveBeenCalledWith(expectedValue);
  });
});

Error output/screenshots

No response

ESLint configuration

module.exports = {
  env: {
    es6: true,
  },
  parserOptions: {
    ecmaVersion: 2021,
    ecmaFeatures: {
      jsx: true,
    },
    sourceType: 'module',
  },
  globals: {
    process: true,
    module: true,
  },
  extends: ['plugin:prettier/recommended'],
  plugins: ['prettier', 'eslint-plugin-no-only-tests', 'deprecate', 'jest', 'testing-library', 'cypress'],
  overrides: [
    {
      files: ['*.test.jsx', '*.test.js', 'jest/setup-tests.js'],
      extends: ['plugin:jest/recommended', 'plugin:testing-library/react'],
    },
  ],
};

Rule(s) affected

testing-library/await-async-utils

Do you want to submit a pull request to fix this bug?

No

Thanks for reporting @iantocristian! This is a regression introduced in the last version indeed.

Pinging @patriscus in case you can check this issue.

I'm sorry to see that I've caused some issues ๐Ÿ‘€ I had a quick look:

The first example seems to be related to the isAssigningKnownAsyncFunctionWrapper I introduced in my latest commit. Using getDeepestIdentifierNode(node.init)?.name ?? '' actually returns true for expectedValue, since we fallback to the default (''). Interestingly, '' is actually a part of the functionWrapperNames since the getFunctionName(innerFunction) call within the detectAsyncUtilWrapper function pushes an empty string into the array.

I very quickly added a safeguard to not push empty strings into the array, and the tests seem to pass (need to verify it a bit more thoroughly).

The second example is more complex in my eyes. This code fails even before my commit - my theory is that since we push wrapper names, we don't really check scopes. So in the example provided:

const waitWithAct = async (timeout) => {
  const fn = async () => await wait(timeout);
  await act(fn);
};

We will push fn into the functionWrapperNames. Later, by using jest.fn(), it's mistakenly flagged as a wrapper around a util because fn matches. So I guess we need to somehow get the "true" variables/references, instead of "just" comparing names.

I could be wrong, though.

I will try to provide a PR for the first issue by tomorrow's EOD.

About the second example, I don't know if it makes sense to do it in this issue. It seems to me that this is a separate topic; hence I wanted to ask if it should be extracted into a separate issue. I will for sure not be able to solve it within the next 2 days, and I'm not available for a couple of days (vacation). I can, however, pick it up after I'm back (would just give a quick ping to let you know).

Please let me know what you think @Belco90.

Thanks for addressing this issue!

@patriscus Better to address the issues in separate PRs!

๐ŸŽ‰ This issue has been resolved in version 5.10.2 ๐ŸŽ‰

The release is available on:

Your semantic-release bot ๐Ÿ“ฆ๐Ÿš€

๐ŸŽ‰ This issue has been resolved in version 6.0.0-alpha.15 ๐ŸŽ‰

The release is available on:

Your semantic-release bot ๐Ÿ“ฆ๐Ÿš€