1162961 - Lots of `menu.getItemByValue(...) is undefined` logspam showing up in dt tests

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect)

Description

[Brian Grinstead [:bgrins]]

See the orange Windows 7 dt runs here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1681803bd065.  Looks like if there is another failure then a bunch of logspam from the netmonitor shows up.

This is somewhat recent (last few days), but I haven't narrowed it down to a single commit.

Comment 1

[Brian Grinstead [:bgrins]]

The first one I see on treeherder on fx-team is: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=067e21985d8d.  However, the previous push (which just doesn't happen to have any failures) looks more suspicious: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=2bda9fb3b5d6.

Guessing this started in Bug 862341 - Panos, can you see if that's right?

Comment 2

[Brian Grinstead [:bgrins]]

Yeah, I can see the following messages even on the green linux opt dt from: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=2bda9fb3b5d6:


TypeError: menu.getItemByValue(...) is undefined: maybeResolve@chrome://mochitests/content/browser/browser/devtools/netmonitor/test/head.js:247:15
TypeError: menu.getItemByValue(...) is undefined: maybeResolve@chrome://mochitests/content/browser/browser/devtools/netmonitor/test/head.js:247:15
TypeError: menu.getItemByValue(...) is undefined: maybeResolve@chrome://mochitests/content/browser/browser/devtools/netmonitor/test/head.js:247:15
TypeError: menu.getItemByValue(...) is undefined: maybeResolve@chrome://mochitests/content/browser/browser/devtools/netmonitor/test/head.js:247:15
TypeError: menu.getItemByValue(...) is undefined: maybeResolve@chrome://mochitests/content/browser/browser/devtools/netmonitor/test/head.js:247:15
TypeError: menu.getItemByValue(...) is undefined: maybeResolve@chrome://mochitests/content/browser/browser/devtools/netmonitor/test/head.js:247:15
TypeError: menu.getItemByValue(...) is undefined: maybeResolve@chrome://mochitests/content/browser/browser/devtools/netmonitor/test/head.js:247:15
TypeError: menu.getItemByValue(...) is undefined: maybeResolve@chrome://mochitests/content/browser/browser/devtools/netmonitor/test/head.js:247:15
TypeError: menu.getItemByValue(...) is undefined: maybeResolve@chrome://mochitests/content/browser/browser/devtools/netmonitor/test/head.js:247:15
TypeError: menu.getItemByValue(...) is undefined: maybeResolve@chrome://mochitests/content/browser/browser/devtools/netmonitor/test/head.js:247:15
TypeError: menu.getItemByValue(...) is undefined: maybeResolve@chrome://mochitests/content/browser/browser/devtools/netmonitor/test/head.js:247:15
TypeError: menu.getItemByValue(...) is undefined: maybeResolve@chrome://mochitests/content/browser/browser/devtools/netmonitor/test/head.js:247:15

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

Could be, I'll take a look on Monday when I'm back from OSCAL.

Comment 4

[Brian Grinstead [:bgrins]]

I'm guessing what's happening is that the request updating events are firing before the netmonitor view has been created, so when it's trying to dig into the view to get the URL it's failing: https://dxr.mozilla.org/mozilla-central/source/browser/devtools/netmonitor/test/head.js#242.  We should probably pass along the URL in the update events so it doesn't rely on that.

Comment 5

[Brian Grinstead [:bgrins]]

Attachment: netmonitor-logspam.patch

Unfortunately can't push to try at the moment because the tree is closed, but this fixes the problem locally for me

Comment 6

[Brian Grinstead [:bgrins]]

Ongoing try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=023dc76034d8

Comment 7

[Brian Grinstead [:bgrins]]

Oh and by the way, if you like the patch and the try push looks good, feel free to check it in so it has a chance to make it into 40 without uplift.

Comment 8

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8603884 [details] [diff] [review]
netmonitor-logspam.patch

Review of attachment 8603884 [details] [diff] [review]:
-----------------------------------------------------------------

Passing an object instead of an array I think would be cleaner and more future-proof, but since this is just for tests it's not a big deal. Thanks for fixing this!

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/05cafe147c0c

Comment 10

[Brian Grinstead [:bgrins]]

(In reply to Panos Astithas [:past] from comment #8)
> Passing an object instead of an array I think would be cleaner and more
> future-proof, but since this is just for tests it's not a big deal.

I think I'll need to touch this again in Bug 1143224, so I'll switch it to an object there.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

This missed the uplift, so please nominate for Aurora approval when you get a chance too.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/05cafe147c0c

Comment 13

[Brian Grinstead [:bgrins]]

Comment on attachment 8603884 [details] [diff] [review]
netmonitor-logspam.patch

Approval Request Comment
[Feature/regressing bug #]: 862341
[User impact if declined]: Log spam showing up in mochitest-dt runs
[Describe test coverage new/current, TreeHerder]: Test only fix
[Risks and why]: Low, test only fix
[String/UUID change made/needed]:

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8603884 [details] [diff] [review]
netmonitor-logspam.patch

Approved for uplift to aurora. This has baked on m-c for a while and should fix some test issues.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9b959bab2b04