Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

New mocha-no-side-effect-code rule #85

Closed
loicraux opened this issue Dec 10, 2015 · 5 comments
Closed

New mocha-no-side-effect-code rule #85

loicraux opened this issue Dec 10, 2015 · 5 comments
Labels
Microsoft Internal Issues related to closed source Microsoft code.
Milestone

Comments

@loicraux
Copy link
Contributor

tl;dr: This rule to ensure no assignment statements or function calls in tests outside mocha before(), after(), beforeEach(), afterEach() and it() scopes.

Reason: You cannot predict (or you are not supposed to know as a developer writing tests) when code containing assignment statements or function calls in tests outside mocha before(), after(), beforeEach(), afterEach() and it() is indeed executed (this is up to requirejs, order of requires for the tests files and also up to Mocha for execution of describe().
This leads to some tests having side effects to some other tests.

With this rule, only variable or function declarations would be allowed outside the scopes listed above.
(Also imports would be allowed)

@HamletDRC HamletDRC added the Microsoft Internal Issues related to closed source Microsoft code. label Aug 12, 2016
@HamletDRC HamletDRC added this to the 2.0.10 milestone Aug 12, 2016
@HamletDRC
Copy link
Member

@s-timofte @loicraux I ran this rule on /common and it found some interesting stuff. I think we could enable this rule fairly easily and I think the cleanup of the test cases would benefit us.

I configured the rule with:

"mocha-no-side-effect-code": [true, { ignore: '^(RestDataFactory\\.create|sinon\.spy).*' }],

@johnemau
Copy link

johnemau commented Mar 9, 2018

What is the guidance for writing dynamically generated tests and enabling this rule?

@kkor
Copy link

kkor commented Nov 8, 2018

Same question here, according to Mochas documentation you can generate tests dynamically like this:
https://mochajs.org/#dynamically-generating-tests

But this will trigger mocha-no-side-effect-code.

@JoshuaKGoldberg
Copy link

@johnemau @kkor good question. It's pretty difficult to tell statically whether side effect code is legitimately setting up dynamic tests or otherwise doing dangerous work. If you have a proposal for how we could determine it, it'd be great if you could file an issue with details.

Until then your best bet is to just disable the rule, either in your tslint.json or with // tslint:disable:mocha-no-side-effect-code` lines.

@WillSquire
Copy link

The thing with Mocha and Jest's way of preparing tests with before(), beforeAll(), etc, is that it involves using var or let to perform variable reassignment. If the idea is to be more functional and avoid side effects, then this rule seems to be a bit of a contradiction to itself?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Microsoft Internal Issues related to closed source Microsoft code.
Projects
None yet
Development

No branches or pull requests

6 participants