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

Allow this.timeout in mocha-no-side-effect-code #446

Closed
astorije opened this issue Jun 27, 2018 · 6 comments
Closed

Allow this.timeout in mocha-no-side-effect-code #446

astorije opened this issue Jun 27, 2018 · 6 comments
Labels
Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. Good First Issue 🙌 Howdy, neighbor! Resolution: Fixed Hooray! Type: Rule Feature Adding a feature to an existing rule.
Milestone

Comments

@astorije
Copy link
Contributor

astorije commented Jun 27, 2018

this.timeout() is valid at suite-level but by default, mocha-no-side-effect-code says:

ERROR: 28:3  mocha-no-side-effect-code  Mocha test contains dangerous variable initialization. Move to before()/beforeEach(): this.timeout(5000)

At the moment, we have to ignore it using the following, but it would be nice if the rule was whitelisting it itself:

mocha-no-side-effect-code:
  - true
  - ignore: 'this\.timeout'

According to the docs, it seems this.retries() and this.slow() should get the same preferential treatment.

@JoshuaKGoldberg
Copy link

Sounds like there should be a whitelist as a config setting!

@JoshuaKGoldberg JoshuaKGoldberg added Status: Accepting PRs Difficulty: Medium People with non-trivial experience in TSLint should be able to send a pull request for this issue. Type: Rule Feature Adding a feature to an existing rule. labels Jul 4, 2018
@astorije
Copy link
Contributor Author

astorije commented Jul 4, 2018

That's what ignore is for, no?
My suggestion here was to ignore this.timeout, this.retries, and this.slow as these are allowed by Mocha itself. What do you think about this?

@JoshuaKGoldberg
Copy link

Ah, you're right! I missed that the rule has that as a config setting. Thanks 😁

It should be easy to add a default ignore value to the rule that's a regex containing those values.

@JoshuaKGoldberg JoshuaKGoldberg added Good First Issue 🙌 Howdy, neighbor! Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. and removed Difficulty: Medium People with non-trivial experience in TSLint should be able to send a pull request for this issue. labels Jul 4, 2018
@astorije
Copy link
Contributor Author

astorije commented Jul 4, 2018

Where would you add this? In the recommended preset, or in the code?

@JoshuaKGoldberg
Copy link

Great question. I suppose that's a preference... though I'd lean towards code, as many don't use the recommended preset.

@astorije
Copy link
Contributor Author

astorije commented Jul 5, 2018

@JoshuaKGoldberg, I gave it a shot in #453!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Difficulty: Easy Someone with little to no experience in TSLint should be able to send a pull request for this issue. Good First Issue 🙌 Howdy, neighbor! Resolution: Fixed Hooray! Type: Rule Feature Adding a feature to an existing rule.
Projects
None yet
Development

No branches or pull requests

2 participants