Closed Bug 1443661 Opened 6 years ago Closed 6 years ago

Enable ESLint rule no-undef for test files in devtools/client/webconsole/new-console-output/

Categories

(DevTools :: Console, enhancement)

Product:
DevTools
Component:
Console
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox61 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 61
Tracking Flags:
Tracking Status
firefox61 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(2 files, 1 obsolete file)

Bug 1443661 - Remove unnecessary ESLint import-globals-from directives from devtools/client/webconsole/new-console-output/test/mochitest.
59 bytes, text/x-review-board-request
nchevobbe
: review+
Details
Bug 1443661 - Enable ESLint rule no-undef for test files in devtools/client/webconsole/new-console-output/.
59 bytes, text/x-review-board-request
nchevobbe
: review+
Details 
no-undef is a very useful rule to have, as part of bug 1230193 we should continue the roll-out.

I have a patch for this.
Another bitrot update...
Attachment #8956652 - Flags: review?(pbrosset) → review?(nchevobbe)
Attachment #8956653 - Flags: review?(pbrosset) → review?(nchevobbe)
This is mostly about the console, I'd rather have Nicolas look at these changes.
Comment on attachment 8956653 [details]
Bug 1443661 - Remove unnecessary ESLint import-globals-from directives from devtools/client/webconsole/new-console-output/test/mochitest.

https://reviewboard.mozilla.org/r/225614/#review234738

Looks good to me (sorry for the delay)
Only need to change the reviewer's nick in the commit message.

::: commit-message-49440:1
(Diff revision 4)
> +Bug 1443661 - Remove unnecessary ESLint import-globals-from directives from devtools/client/webconsole/new-console-output/test/mochitest. r=pbro

s/pbro/nchevobbe
Attachment #8956653 - Flags: review?(nchevobbe) → review+
Comment on attachment 8956652 [details]
Bug 1443661 - Enable ESLint rule no-undef for test files in devtools/client/webconsole/new-console-output/.

https://reviewboard.mozilla.org/r/225612/#review234742

::: commit-message-a007d:1
(Diff revision 4)
> +Bug 1443661 - Enable ESLint rule no-undef for test files in devtools/client/webconsole/new-console-output/. r=pbro

s/pbro/nchevobbe

::: devtools/.eslintrc.mochitests.js:23
(Diff revision 4)
> +    // Allow non-camelcase so that run_test doesn't produce a warning.
> +    "camelcase": "off",

not sure if doable, but could we let it on and add an exception for `run_test` ?

::: devtools/client/webconsole/new-console-output/test/components/.eslintrc.js:4
(Diff revision 4)
> +"use strict";
> +
> +module.exports = {
> +  "extends": "../.eslintrc.mocha.js"

it looks a bit weird to me to extend the mocha configuration here. Do we need the same things ?

::: devtools/client/webconsole/new-console-output/test/middleware/.eslintrc.js:3
(Diff revision 4)
> +module.exports = {
> +  "extends": "../.eslintrc.mocha.js"
> +};

same thing here, I wouldn't expect to have the same needs than in the mocha env

::: devtools/client/webconsole/new-console-output/test/mochitest/browser_console_webconsole_private_browsing.js:8
(Diff revision 4)
> +// XXX Remove this when the file is migrated to the new frontend.
> +/* eslint-disable no-undef */

the test is now migrated (landed in autoland, not in mc yet)

::: devtools/client/webconsole/new-console-output/test/store/.eslintrc.js:3
(Diff revision 4)
> +module.exports = {
> +  "extends": "../.eslintrc.mocha.js"
> +};

same here about extending the mocha configuration

::: devtools/client/webconsole/new-console-output/test/utils/.eslintrc.js:3
(Diff revision 4)
> +module.exports = {
> +  "extends": "../.eslintrc.mocha.js"
> +};

same here about extending the mocha configuration
Comment on attachment 8956652 [details]
Bug 1443661 - Enable ESLint rule no-undef for test files in devtools/client/webconsole/new-console-output/.

https://reviewboard.mozilla.org/r/225612/#review234742

> not sure if doable, but could we let it on and add an exception for `run_test` ?

Unfortunately camelcase doesn't give that option https://eslint.org/docs/rules/camelcase

> it looks a bit weird to me to extend the mocha configuration here. Do we need the same things ?

We do need it. What it is effectively doing, is saying that the configuration applies to this directory, and that we're sharing that configuration file across several directories. We don't want to apply it for all of devtools/client/webconsole/new-console-output/test/ as that would likely mean incorrect globals being allowed causing problems for no-undef & other rules.

Note that `.eslintrc.mocha.js` isn't a `.eslintrc.js` file, so isn't applied within the upper level directory.
Attachment #8956652 - Attachment is obsolete: true
Attachment #8956652 - Flags: review?(nchevobbe)
Sorry, I (or maybe git) messed up the rebase, trying to fix at the moment.
Comment on attachment 8960577 [details]
Bug 1443661 - Enable ESLint rule no-undef for test files in devtools/client/webconsole/new-console-output/.

https://reviewboard.mozilla.org/r/229320/#review235374
Attachment #8960577 - Flags: review?(nchevobbe) → review+
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4e5fb7ed54de
Enable ESLint rule no-undef for test files in devtools/client/webconsole/new-console-output/. r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/57e2de4f048e
Remove unnecessary ESLint import-globals-from directives from devtools/client/webconsole/new-console-output/test/mochitest. r=nchevobbe
Backed out of ESlint failure on test_websocket-server.html:50

backout link: https://hg.mozilla.org/integration/autoland/rev/cf2202675264a1e22c33b21952f65adc280447fb

push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=57e2de4f048e63c16279b373a4fd413613a546f8&selectedJob=169385283

failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=169385283&repo=autoland&lineNumber=266

[task 2018-03-21T07:21:28.939Z] executing ['bash', '-cx', 'cd /builds/worker/checkouts/gecko/ && cp -r /build/node_modules_eslint node_modules && ln -s ../tools/lint/eslint/eslint-plugin-mozilla node_modules && ln -s ../tools/lint/eslint/eslint-plugin-spidermonkey-js node_modules && ./mach lint -l eslint -f treeherder --quiet\n']
[task 2018-03-21T07:21:28.948Z] + cd /builds/worker/checkouts/gecko/
[task 2018-03-21T07:21:28.948Z] + cp -r /build/node_modules_eslint node_modules
[task 2018-03-21T07:21:29.389Z] + ln -s ../tools/lint/eslint/eslint-plugin-mozilla node_modules
[task 2018-03-21T07:21:29.390Z] + ln -s ../tools/lint/eslint/eslint-plugin-spidermonkey-js node_modules
[task 2018-03-21T07:21:29.391Z] + ./mach lint -l eslint -f treeherder --quiet
[task 2018-03-21T07:21:30.095Z] New python executable in /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenv/bin/python2.7
[task 2018-03-21T07:21:30.096Z] Also creating executable in /builds/worker/checkouts/gecko/obj-x86_64-pc-linux-gnu/_virtualenv/bin/python
[task 2018-03-21T07:21:31.525Z] Installing setuptools, pip, wheel...done.
[task 2018-03-21T07:21:32.579Z] running build_ext
[task 2018-03-21T07:21:32.579Z] building 'psutil._psutil_linux' extension
[task 2018-03-21T07:21:32.579Z] creating build
[task 2018-03-21T07:21:32.579Z] creating build/temp.linux-x86_64-2.7
[task 2018-03-21T07:21:32.579Z] creating build/temp.linux-x86_64-2.7/psutil
[task 2018-03-21T07:21:32.580Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2018-03-21T07:21:32.580Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2018-03-21T07:21:32.580Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_linux.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o
[task 2018-03-21T07:21:32.580Z] creating build/lib.linux-x86_64-2.7
[task 2018-03-21T07:21:32.580Z] creating build/lib.linux-x86_64-2.7/psutil
[task 2018-03-21T07:21:32.581Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o build/temp.linux-x86_64-2.7/psutil/_psutil_linux.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so
[task 2018-03-21T07:21:32.581Z] building 'psutil._psutil_posix' extension
[task 2018-03-21T07:21:32.581Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_common.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_common.o
[task 2018-03-21T07:21:32.582Z] x86_64-linux-gnu-gcc -pthread -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fno-strict-aliasing -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -fPIC -DPSUTIL_POSIX=1 -DPSUTIL_VERSION=543 -DPSUTIL_LINUX=1 -I/usr/include/python2.7 -c psutil/_psutil_posix.c -o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o
[task 2018-03-21T07:21:32.582Z] x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security -Wl,-Bsymbolic-functions -Wl,-z,relro -Wdate-time -D_FORTIFY_SOURCE=2 -g -fstack-protector-strong -Wformat -Werror=format-security build/temp.linux-x86_64-2.7/psutil/_psutil_common.o build/temp.linux-x86_64-2.7/psutil/_psutil_posix.o -o build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so
[task 2018-03-21T07:21:32.582Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_linux.so -> psutil
[task 2018-03-21T07:21:32.582Z] copying build/lib.linux-x86_64-2.7/psutil/_psutil_posix.so -> psutil
[task 2018-03-21T07:21:32.582Z] 
[task 2018-03-21T07:21:32.582Z] Error processing command. Ignoring because optional. (optional:packages.txt:comm/build/virtualenv_packages.txt)
[task 2018-03-21T07:26:08.184Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/server/socket/tests/test_websocket-server.html:50:5 | 'SimpleTest' is not defined. (no-undef)
[task 2018-03-21T07:26:08.184Z] TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/server/socket/tests/test_websocket-server.html:77:5 | 'is' is not defined. (no-undef)
[taskcluster 2018-03-21 07:26:08.501Z] === Task Finished ===
[taskcluster 2018-03-21 07:26:08.501Z] Unsuccessful task run with exit code: 1 completed in 537.711 seconds
Flags: needinfo?(standard8)
Minor change needed due to bug 1446222 landing on autoland (and not having got the right reference in the .eslintrc.js file that it added).
Flags: needinfo?(standard8)
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6fadc412efcb
Enable ESLint rule no-undef for test files in devtools/client/webconsole/new-console-output/. r=nchevobbe
https://hg.mozilla.org/integration/autoland/rev/5b4406c6491f
Remove unnecessary ESLint import-globals-from directives from devtools/client/webconsole/new-console-output/test/mochitest. r=nchevobbe
https://hg.mozilla.org/mozilla-central/rev/6fadc412efcb
https://hg.mozilla.org/mozilla-central/rev/5b4406c6491f
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: