code-review
code-review copied to clipboard
Test manifest warnings don't appear on phabricator output
This phabricator revision didn't indicate that there were any warnings from the linters.
However, looking at the logs from the treeherder run, there were warnings given. In this case it was:
TEST-UNEXPECTED-WARNING | /builds/worker/checkouts/gecko/toolkit/mozapps/extensions/test/xpcshell/xpcshell.toml:0 | The manifest sections are not in alphabetical order. (test-manifest-toml)
I've seen in the past warnings from the ESLint runs, so I'd have expected this to report an issue as well.
This is not the only instance of missing linting reports; I fixed up many issues that should have been caught by the linter, at https://bugzilla.mozilla.org/show_bug.cgi?id=1953004
I have rebased the patch to https://bugzilla.mozilla.org/show_bug.cgi?id=1953004 a couple of times, and every time a new instance of a failing linting check was introduced. Without enforcement of the check, this is bound to happen more often.
More information from Standard8: Warnings aren't being show in code review bot results.
Test Phabricator revision: https://phabricator.services.mozilla.com/D265816 There's a warning on the line I touched (and several others in the file). The warning on line 17 where the error is, is new. The other ones in the file were pre-existing.
This is another revision, where I accidentally introduced a warning but thankfully my reviewer picked it up.
The warning was reported in the Treeherder log but not reported to Phabricator.
I looked into this issue and found that we get a publishable=False response from the backend for these warnings.
As the backend only check if they are at error level OR in patch, it could be either:
- invalid
in_patchstate reported from the bot onto the backend (too early, before having the real state of in patch ?) - erroneous state for full file issues (on line 0). But this used to work too
I'm trying to find when this behaviour changed
@La0 the warning mentioned in https://github.com/mozilla/code-review/issues/2665#issuecomment-3334296583 was not a full file one:
{"linter": "eslint", "path": "/builds/worker/checkouts/gecko/toolkit/actors/NetErrorParent.sys.mjs", "lineno": 17, "column": 10, "message": "> resource:///modules/GenAI.sys.mjs is part of Desktop Firefox and cannot be unconditionally used by this code (which has to also work elsewhere).", "hint": null, "source": null, "level": "warning", "rule": "mozilla/no-browser-refs-in-toolkit", "lineoffset": null, "diff": null, "relpath": "toolkit/actors/NetErrorParent.sys.mjs"}
@La0 the warning mentioned in #2665 (comment) was not a full file one:
Yes, I think it's a general issue (not only tied to full-file)
This has been resolved.