1780020 - List of xpcshell test paths in .eslintrc.js is incomplete/ needs adjustments

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, task)

Description

[Tomer Yavor]

In .eslintrc.js exists a variable which could be misleading:

const xpcshellTestPaths = [
  "**/test*/unit*/**/",
  "**/test*/*/unit*/",
  "**/test*/xpcshell/**/",
];

https://searchfox.org/mozilla-central/source/.eslintrc.js#24-28.

In mozila-central we have e.g., the test paths testing/profiles/xpcshell/ or browser/components/about/test/unit/ which are not included in the list (note **/test*/unit*/**/ != **/test*/unit*/).

That's seems incomplete, we should consider a update (comment why we exclude paths or additional paths).

Comment 1

[Mark Banner (:standard8)]

(In reply to Tomer Yavor from comment #0)

In mozila-central we have e.g., the test paths testing/profiles/xpcshell/ or browser/components/about/test/unit/ which are not included in the list (note **/test*/unit*/**/ != **/test*/unit*/).

testing/profiles/xpcshell/ is not a folder that contains xpcshell test files - it is only for the profile files that xpcshell tests use in their tests. As they are profile related, we don't need to apply the specific xpcshell test rules there.

browser/components/about/test/unit/ which are not included in the list (note **/test*/unit*/**/ != **/test*/unit*/).

I think that the existing **/test*/unit*/**/ does match browser/components/about/test/unit/. My understanding is that ** will match 0 or more characters and span across directories.

Additionally If I comment out that line from the top-level .eslintrc.js file, then browser/components/about/test/unit/test_getURIFlags.js fails due to missing symbols.

That's seems incomplete, we should consider a update (comment why we exclude paths or additional paths).

We do have documentation in the source docs: https://firefox-source-docs.mozilla.org/code-quality/lint/linters/eslint.html#i-m-adding-tests-how-do-i-set-up-the-right-configuration

Though I do agree that it would be good to add a comment in the file as well.

Comment 2

[Mark Banner (:standard8)]

Attachment: Bug 1780020 - Add a comment to the top-level .eslintrc.js explaining the path formats. r?#linter-reviewers

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6ec2596bc7da
Add a comment to the top-level .eslintrc.js explaining the path formats. r=linter-reviewers,sylvestre

Comment 4

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/6ec2596bc7da

Comment 5

[Tomer Yavor]

I think that the existing /test/unit/**/ does match browser/components/about/test/unit/. My understanding is that ** will match 0 or more characters and span across directories

Hmm... when I used it for the HTTPS linter there are tests that were not excluded.
Also in searchfox it doesn't look equivalent.
for **/test*/unit*/**:
https://searchfox.org/mozilla-central/search?q=&path=**%2Ftest*%2Funit*%2F**&case=false&regexp=false

for **/test*/unit*/**/**:
https://searchfox.org/mozilla-central/search?q=&path=**%2Ftest*%2Funit*%2F**%2F&case=false&regexp=false

But probably it is not relevant for no-unused-vars since it's exempting the rest of xpcshell tests directly or indirectly in https://searchfox.org/mozilla-central/source/.eslintrc.js#203,219-229

Thank you for adding a comment.