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

mocha-no-side-effect-code reported when including/excluding tests #389

Closed
karfau opened this issue Aug 26, 2017 · 6 comments
Closed

mocha-no-side-effect-code reported when including/excluding tests #389

karfau opened this issue Aug 26, 2017 · 6 comments
Labels
Difficulty: Medium People with non-trivial experience in TSLint should be able to send a pull request for this issue. Resolution: Fixed Hooray! Type: Bug
Milestone

Comments

@karfau
Copy link
Contributor

karfau commented Aug 26, 2017

The mocha-no-side-effect-code is also triggered when using one of the following feature of mocha:
http://mochajs.org/#inclusive-tests

e.g. it.skip(... or describe.skip(...

I think this should not happen.

(Update: I removed reference to exclusive tests aka it.only(..., because in that the rule mocha-avoid-only marks the failure, which makes sense in my understanding.)

@NiKlimenko
Copy link

You can configure a rule: "mocha-no-side-effect-code": [true, {"ignore": "it\.skip.*"}]
@loicraux @HamletDRC but it really should be the default, don't you think?

@loicraux
Copy link
Contributor

I am not sure that it should be the default. A deactivated test does not mean its code does not have to pass the linter rules imho... I see only one good reason for using it.skip : when the feature you test is not yet implemented. Sometimes (unfortunately) you also deactivate a broken test because you plan to fix the tested code or the test code later...
But you do not skip a test because its code does not pass the linter rules, don't you think ?

@karfau
Copy link
Contributor Author

karfau commented Oct 18, 2017

@loicraux I'm not sure I'm understanding what you are trying to say.

What I understood from @NiKlimenko's comment is that it might be possible to come to the same behavior by using the suggested configuration. This might or might not hold true, but that configuration might have other side effects, is that what you are trying to say when writing about "not having to pass linter rules"?

Why I filed this issue was that I'm under the impression that using is.skip is not something the mocha-no-side-effect-code rule should mark as an failure.
(In case you disagree it would help me, if you or somebody else could explain why they think it should be marked as being a side effect thing.)

@HamletDRC HamletDRC added this to the 5.0.4 milestone Apr 17, 2018
@HamletDRC
Copy link
Member

Here is the test that I made pass:

it('should pass on usage of skip/only', () : void => { const script : string =
describe('someTest', (): void => {

            it.skip((): void => {
            });

            describe.skip((): void => {
                it.skip((): void => {
                });
            });

            it.only((): void => {
            });
            describe.only((): void => {
                it.only((): void => {
                });
            });
        });
    `;

    TestHelper.assertViolations(ruleName, script, [ ]);
})`

@karfau
Copy link
Contributor Author

karfau commented Apr 17, 2018

🎉 thx for fixing this

@HamletDRC
Copy link
Member

@karfau thanks for taking the time to report it and then being patient for the last few months! It has been a crazy winter and we're un-burying ourselves now.

@JoshuaKGoldberg JoshuaKGoldberg added Type: Bug Difficulty: Medium People with non-trivial experience in TSLint should be able to send a pull request for this issue. Resolution: Fixed Hooray! labels Jul 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Difficulty: Medium People with non-trivial experience in TSLint should be able to send a pull request for this issue. Resolution: Fixed Hooray! Type: Bug
Projects
None yet
Development

No branches or pull requests

5 participants