Ghost: Clean up mocha eslint errors

Created on 19 Aug 2019  路  12Comments  路  Source: TryGhost/Ghost

We've moved to [email protected] which includes eslint-plugin-mocha.

A number of the mocha linting rules are quite valuable for keeping tests performant and readable but were turned off in Ghost because they caused a lot of noise with our existing codebase. It would be great to resolve the issues so that we can turn the rules back on.

You can find the list of rules that were turned off here https://github.com/TryGhost/Ghost/blob/master/core/test/.eslintrc.json#L24-L27

For anyone picking this issue up, a separate PR for each rule would be the preferred approach, please comment on this issue if you make a start to avoid duplicate work 馃檪

General approach:

  1. Choose a rule and check the rule's docs to make sure you understand what it's checking for and how to fix reported problems
  2. Remove the line from core/test/.eslintrc.json that corresponds with the rule that you are enabling
  3. Run yarn lint:test
  4. Work through the reported problems - make a change to the code then run yarn lint:test again to make sure the problem disappears. If you have eslint configured in your editor you should be able to get some immediate feedback too
  5. Periodically run grunt test:unit to make sure there haven't been any non-linting problems introduced by the code changes
good first issue help wanted stale

Most helpful comment

Hi,
@gargol
Updated PR #11154 after I accidentally pulled in some additional code into this commit.
passes

npm run test
npm run lint

Please let me know if anything else needs to be changed!

All 12 comments

starting on rule:
ghost/mocha/max-top-level-suites

working on rule ghost/mocha/no-identical-title

Hi,
created PR #11141
for max-top-level-suites
Thanks!

I'm going to start on the next available one:
no-setup-in-describe

Edit:

This next one will take a bit, there are over 1000 warnings related to this one.

I also reviewed nip10's PR and refactored the two pieces of code he was stuck on.
Once they are fixed it should be ready to be merged.

completed no-setup-in-describe

11154

edit:
Okay, so I was able to get the recent updates that have been completed in the project recently into my pull requests. They pass travis cl and eslint, when I try to squash the commits it gives me the following error at least for PR #11141 it gives me the following error:
Merge conflict in package.json
error: could not apply d33fc8c8a... Update dependency @tryghost/helpers to v1.1.10
and I found this SO question related to having a number of branches active and being unable to squash commits?
https://stackoverflow.com/questions/31069316/error-with-git-rebase-could-not-apply

Hi,

I did a git pull push origin on my local repo.
Completed refactor changes and resubmitted PR
which is now #11164

Please let me know if any changes need to be made!

I am picking ghost/mocha/no-sibling-hooks

Hello! are all the rules fixed? can I pick one?

Hi, I believe they've all been claimed @dsuarezlogans

Hi,
@gargol
Updated PR #11154 after I accidentally pulled in some additional code into this commit.
passes

npm run test
npm run lint

Please let me know if anything else needs to be changed!

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Was this page helpful?
0 / 5 - 0 ratings