List of xpcshell test paths in .eslintrc.js is incomplete/ needs adjustments
Categories
(Developer Infrastructure :: Lint and Formatting, task)
Tracking
(firefox104 fixed)
Tracking | Status | |
---|---|---|
firefox104 | --- | fixed |
People
(Reporter: t.yavor, Assigned: standard8)
Details
Attachments
(1 file)
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).
Assignee | ||
Comment 1•2 years ago
•
|
||
(In reply to Tomer Yavor from comment #0)
In mozila-central we have e.g., the test paths
testing/profiles/xpcshell/
orbrowser/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.
Assignee | ||
Comment 2•2 years ago
|
||
Updated•2 years ago
|
Pushed by mbanner@mozilla.com: 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•2 years ago
|
||
bugherder |
Reporter | ||
Comment 5•2 years ago
•
|
||
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®exp=false
for **/test*/unit*/**/**
:
https://searchfox.org/mozilla-central/search?q=&path=**%2Ftest*%2Funit*%2F**%2F&case=false®exp=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.
Updated•2 years ago
|
Description
•