1328254 - Undefined variables used in browser_aboutperformance.js highlighting unused test code

Status: RESOLVED FIXED

(Toolkit :: Performance Monitoring, defect)

Description

[Mark Banner (:standard8)]

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.

Comment 1

[David Teller [:Yoric] - still alive but not very active]

That's pretty useful, thanks. Deflecting to jaws, who's mentoring someone to rewrite that entire codebase.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Unfortunately we have shifted away from the about:performance project due to a lack of specification and well defined direction.

Comment 3

[David Teller [:Yoric] - still alive but not very active]

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.

Comment 4

[Mark Banner (:standard8)]

Attachment: Bug 1328254 - Fix browser_aboutperformance.js to correctly check the delta displays.

Review commit: https://reviewboard.mozilla.org/r/106070/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/106070/

Comment 5

[Mark Banner (: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 6

[Jared Wein [:jaws] (please needinfo? me)]

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.

Comment 7

[Mark Banner (:standard8)]

(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.

Comment 8

[David Teller [:Yoric] - still alive but not very active]

Thanks a lot.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ac654e1c426
Fix browser_aboutperformance.js to correctly check the delta displays. r=jaws

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/7ac654e1c426