Closed Bug 1328254 Opened 3 years ago Closed 3 years ago
Undefined variables used in browser
_aboutperformance .js highlighting unused test code
59 bytes, text/x-review-board-request
Turning on no-undef for browser_aboutperformance.js, gives the following eslint errors: toolkit/components/aboutperformance/tests/browser/browser_aboutperformance.js 109:60 error 'eltDetails' is not defined. no-undef (eslint) 122:47 error 'eltDetails' is not defined. no-undef (eslint) 122:69 error 'reCPU' is not defined. no-undef (eslint) 129:61 error 'eltDetails' is not defined. no-undef (eslint) (there's some others about sendAsyncMessage/addMessageListener which are not related to this bug). The reCPU issue needs this line adding back in, that I accidentally removed in bug 1315951 (the case of the variable name was wrong): let reCPU = /CPU usage: (\d+)%/; The eltDetails appear to be actual issues, but when investigating those, it became clear that the ` addMessageListener("aboutperformance-test:checkSanity",` section of the test isn't being fully run. The `content.document.querySelectorAll("delta")` for loop appears to not iterate - presumably, the delta isn't found, or it doesn't have any items: https://dxr.mozilla.org/mozilla-central/rev/c91249f41e3766274131a84f9157a4d9d9949520/toolkit/components/aboutperformance/tests/browser/browser_aboutperformance.js#90 This code was added in bug 1189799.
That's pretty useful, thanks. Deflecting to jaws, who's mentoring someone to rewrite that entire codebase.
Flags: needinfo?(dteller) → needinfo?(jaws)
Unfortunately we have shifted away from the about:performance project due to a lack of specification and well defined direction.
Flags: needinfo?(jaws) → needinfo?(dteller)
So, thanks for the bug reports. Unfortunately, I'm not working on Firefox at all these days, so I don't know when I would find time to fix these things. I will try to find time at some point in February, if my CD deadlines don't press too much, but I can make no promise.
I found the issue with the missing iteration & fixed that, and then also fixed a few other issues that the test showed up. I'll do a try push just to make sure there's no intermittents introduced with this.
Comment on attachment 8828790 [details] Bug 1328254 - Fix browser_aboutperformance.js to correctly check the delta displays. https://reviewboard.mozilla.org/r/106070/#review107088 Thank you for fixing these. I'm not sure how this test was running/passing before.
Attachment #8828790 - Flags: review?(jaws) → review+
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6) > Comment on attachment 8828790 [details] > Thank you for fixing these. I'm not sure how this test was running/passing > before. It didn't have the right item for query selector call, so the section didn't run - I added a check for making sure there's at least one item in the array so it can't happen in future.
Thanks a lot.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/7ac654e1c426 Fix browser_aboutperformance.js to correctly check the delta displays. r=jaws
You need to log in before you can comment on or make changes to this bug.