Closed Bug 1328254 Opened 3 years ago Closed 3 years ago

Undefined variables used in browser_aboutperformance.js highlighting unused test code

Categories

(Toolkit :: Performance Monitoring, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

Details

Attachments

(1 file)

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.
Flags: needinfo?(dteller)
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.
Flags: needinfo?(dteller)
Assignee: nobody → standard8
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.
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7ac654e1c426
Fix browser_aboutperformance.js to correctly check the delta displays. r=jaws
https://hg.mozilla.org/mozilla-central/rev/7ac654e1c426
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.