1612907 - [meta] Enable ESLint rule no-unused-vars globally on xpcshell-test files

Status: RESOLVED FIXED

(Toolkit :: General, task, P3)

Description

[Mark Banner (:standard8)]

I thought we already had no-unused-vars enabled for the global scope as well as local on xpcshell-tests, but it appears that we only have it enabled for the local scope.

To help reduce noise in our tests, ensure that implementers aren't forgetting part of a test, and help maintenance, we should enable no-unused-vars in the global scope of xpcshell-test files, but not for the head files.

Excluding the head files part might also want bug 1447034 as that should help reduce the false-positives.

Comment 1

[Mark Banner (:standard8)]

I'm happy to mentor work towards this bug. Please don't ask to be assigned to it - file a new bug as described below.

Here's what to do:

• Make sure you have a local build of Firefox up and running. Use these instructions if you haven't already.
  • If you need help setting up, ask in #introduction on Matrix
• Next, here are the files that need fixing.
  • Open up the top-level .eslintrc.js file in your editor and find those lines. Some may already be fixed, so focus on ones that are still listed.
  • Check the dependencies of this bug to see if they are already in progress, if they are, please leave them to that bug owner.
  • Select one or two lines to fix - if you take a couple of lines, make sure they are in the same area of code.
• Once you've found one or two lines to fix, file a bug:
  • Use ./mach file-info bugzilla-component <path/to/file> to find the product and component to file the bug under.
  • As you file the bug:
    • Show advanced fields
    • assign it to yourself
    • add this bug (1612907) in the "Blocks" field.
    • Add a summary such as "Fix ESLint rule warnings for no-unused-vars in path/to/file". A brief description may be nice too!
• Now you can fix the issue:
  • Remove the chosen lines from the top-level .eslintrc.js file.
  • Run ./mach eslint path/to/file to see where the issues are.
  • Fix the ESLint issues - generally this should be removing the unused variable definition, e.g. if it is var foo = 5; and foo is not referenced anywhere, then you can remove the whole line.
  • Then re-run ./mach eslint path/to/file to check it now passes.
• Run the test to check it still passes, e.g. ./mach xpcshell-test path/to/file
• Now create a commit with a message such as "Bug nnnnnn - Fix ESLint rule warnings for no-unused-vars in path/to/file. r?Standard8"
• Use moz-phab submit to submit the patch for review.

Comment 2

[Roy Christo]

Attachment: Bug 1612907 - Fix ESLint rule warnings for no-unused-vars in xpcom/tests/unit/test_localfile.js. r?Standard8

Comment 3

[Phabricator Automation]

Comment on attachment 9270609 [details]
Bug 1612907 - Fix ESLint rule warnings for no-unused-vars in xpcom/tests/unit/test_localfile.js. r?Standard8

Revision D142764 was moved to bug 1762749. Setting attachment 9270609 [details] to obsolete.

Comment 4

[Dmitrij Feller]

Here you can check how to set up moz-phab to submit the patch.

Comment 5

[Mark Banner (:standard8)]

This is now complete.