onechiporenko / eslint-plugin-mocha-cleanup Goto Github PK

I have found an issue with invalid-assertions. Arrow functions without parenthesis or curly braces around the body are not detected as invalid assertions.

Does not work:

Object.entries(myReference).forEach(([key, value]) =>
        it(`prints ${key}`, () => expect(pageData.includes(value)) ),
      );

Works:

Object.entries(myReference).forEach(([key, value]) =>
        it(`prints ${key}`, () => { expect(pageData.includes(value)) }),
      );

Works:

Object.entries(getStatementConfig()).forEach(([key, value]) =>
        it(`prints ${key}`, () => ( expect(pageData.includes(value)) )),
      );

versions:

"node": "v14.16.0"
"vscode": "1.60.2"
"eslint": "^7.20.0"
"eslint-plugin-mocha-cleanup": "^1.9.1"

Doing variable assignment directly within describe/context callbacks (outside of it callbacks) makes said variables accessible to the tests through closures. As such, the variables create test pollution as the variables are shared between tests. Generally, the solution to this is to assign these variables to the context object that mocha creates for each test (which is bound as this within beforeEach/it/afterEach callbacks). However, ensuring that the variables are only declared within the describe callbacks and that they are assigned within beforeEach/it callbacks is usually sufficient. (usually)

I propose a rule that finds any variables that are assigned within describe/context callbacks. I intend to work on a PR for this feature. Indeed, I was planning on creating an eslint plugin specifically for this rule, but figured that it would be quite at home in this plugin instead.

I've found an issue with no-empty-body rule:

it('test', () => expect('a').to.equal('a'));

The rule complains about an empty body, when it isn't, as you can see on the provided example.

Hi,

It'd be nice to avoid needing to add to plugins and manually adding best practice rules (instead just needing to add extends: ['plugin:mocha-cleanup/recommended'] and enabling/overriding any additional rules desired). Would you be open to a recommended config?

I wouldn't think it should have specific assert limits or complexity limits by default as I'd think these would vary more by project, but some rules like no-empty-title, just seem good to get by default.

I could offer a PR if this were desired. Thanks!

Hi,

As of ESLint 8, there is now a new format for ESLint config files which requires that plugins follow a specific format. In ESLint 9, this format will be the only one available by default (and other API changes have occurred in ESLint 9 that may affect this plugin).

Info:

1. https://eslint.org/docs/latest/use/configure/configuration-files-new
2. https://eslint.org/blog/2022/08/new-config-system-part-1/
3. https://eslint.org/blog/2022/08/new-config-system-part-2/
4. https://eslint.org/docs/latest/use/configure/migration-guide

Thanks!

Somente tests where Sinon is imported with the name "Sinon" instead "sinon" returns Test without assertions is not allowed.

e.g.

const rule = require('../../../lib/rules/asserts-limit');
const { RuleTester } = require('eslint');

const ruleTester = new RuleTester({ env: { es6: true } });

ruleTester.run('asserts-limit', rule, {
  valid: [
    {
      only: true,
      code: `it('1234', function () {
        Sinon.assert.calledOn(sp, {});
      })`
    }
  ],
  invalid: [],
});

It appears that changing lib/utils/node.js#L36
from node.object.name === 'sinon' to String(node.object.name).toLowerCase() === 'sinon' solves.