1426882 - set up eslint rules in /mailnews xpcshell tests

Status: RESOLVED DUPLICATE of bug 1515877

(Thunderbird :: Build Config, defect)

Description

[:aceman]

+++ This bug was initially created as a clone of Bug #1426459 +++

There are already predefined rules for xpcshell tests at
mozilla/tools/lint/eslint/eslint-plugin-mozilla/lib/configs/xpcshell-test.js, mainly silencing warnings about xpcshell global functions whose declarations can't be seen by the linter. Let's use it for mailnews tests.

Comment 1

[:aceman]

Attachment: 1426882.patch

Try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f82a79dda31beaefb1387ec755b11ef9f3aa4e95

Comment 2

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8938676 [details] [diff] [review]
1426882.patch

Review of attachment 8938676 [details] [diff] [review]:
-----------------------------------------------------------------

This is generally ok so r+, but please only land this if the lint target remains green and the directories you added .eslintrc.js to are actually included in the lint run.

Comment 3

[:aceman]

Well, none of that is true now and can hardly be satisfied.
This is meant to be a baseline setting on which further bugs cleaning up individual tests/directories can build.

Comment 4

[Philipp Kewisch [:Fallen][📱📆]]

In that case I think it makes more sense to add this once the directories are cleaned. There is no use adding this config if the directories are not run. Possibly WONTFIX and then open more specific bugs to clean up the test directories?

Comment 5

[:aceman]

For me it does not make sense. Anybody (including me) in the future will have to wonder about the many useless warnings, remember to include this predefined file and only then start to fix real problems. For me it makes more sense to land this when we already thought about this and save time in the future.

Comment 6

[Philipp Kewisch [:Fallen][📱📆]]

So then maybe turn it around: create an /mailnews/.eslintrc.js that disables all failing rules explicitly, with a comment that these are to be enabled later, then remove mailnews from eslintignore. This way the directory is run, and you can enable the remaining rules gradually.

Given we disagree about the approach I'm happy to listen to a third opinion though.

Comment 7

[Tom Prince [:tomprince]]

I think putting FIXME's in the lint configuration is the right solution. That is what is done other places: https://dxr.mozilla.org/mozilla-central/source/taskcluster/.yamllint#10

Comment 8

[:aceman]

Geoff, could you finish this when you get some time?

Comment 9

[Geoff Lankow (:darktrojan) [PTO, back 28/4]]

Eventually. When I've finished mail/components I'm aiming to do mail/base. Maybe after that?