Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add settings to enable support for mocha.parallel #126

Merged
merged 1 commit into from
Mar 17, 2017

Conversation

stefanbuck
Copy link
Contributor

@stefanbuck stefanbuck commented Mar 15, 2017

This PR adds a new shared settings value which accepts an array of additional suite names, for example:

"settings": { "mocha/additionalSuiteNames': [ 'foo', 'bar' ], }

This is useful when working with modules like mocha.parallel

@lo1tuma let me know what you think and I'll adapt it for the following rules as well.

  • no-top-level-hooks
  • max-top-level-suites
  • no-hooks-for-single-case
  • no-identical-title
  • no-nested-tests
  • no-sibling-hooks

@stefanbuck stefanbuck force-pushed the parallel-support branch 2 times, most recently from 53521d1 to 4730ac8 Compare March 16, 2017 06:22
@lo1tuma
Copy link
Owner

lo1tuma commented Mar 16, 2017

Thanks for the PR.

Actually I was thinking about a more generic approach. I would prefer a setting which accepts an array of additional suite names, for example:

"settings": { "mocha/additionalSuiteNames": [ "parallel", "xparallel", "myOtherMochaExtensionCustomSuiteName" ] } }

This way we would be agnostic to any specific mocha extension and we don’t have to add a separate setting for each custom suite name. What do you think?

@stefanbuck stefanbuck force-pushed the parallel-support branch 2 times, most recently from d4fefc9 to b42a96a Compare March 16, 2017 18:15
@stefanbuck
Copy link
Contributor Author

Yeah, make sense. I've updated the PR as you suggested. Let me know if it's okay for you and I'll tweak the other rules as well.

Copy link
Owner

@lo1tuma lo1tuma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work 👍, LGTM.

@stefanbuck
Copy link
Contributor Author

@lo1tuma please have another look. I'm undecided if the valid test cases really make sense, since the suite names is optional.

@lo1tuma
Copy link
Owner

lo1tuma commented Mar 17, 2017

@stefanbuck I think those tests make sense, if someone breaks the feature of addtionalSuteNames those tests would fail.

@lo1tuma lo1tuma merged commit 6721d2e into lo1tuma:master Mar 17, 2017
@stefanbuck stefanbuck deleted the parallel-support branch March 17, 2017 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants