1945663 - browser/components/urlbar/tests/quicksuggest/browser/browser_quicksuggest_ping_amp.js perma-fails for artifact builds

Status: RESOLVED FIXED

(Firefox :: Address Bar, task, P1)

Description

[:Gijs (he/him)]

Sample job

Sample log

[task 2025-02-03T13:50:42.721Z] 13:50:42     INFO - TEST-PASS | browser/components/urlbar/tests/quicksuggest/browser/browser_quicksuggest_ping_amp.js | Sanity check: The expected ping should have a 'pingType' - "quicksuggest-impression" == true - 
[task 2025-02-03T13:50:42.727Z] 13:50:42     INFO - TEST-PASS | browser/components/urlbar/tests/quicksuggest/browser/browser_quicksuggest_ping_amp.js | Sanity check: The expected ping should have key: pingType - true == true - 
[task 2025-02-03T13:50:42.733Z] 13:50:42     INFO - TEST-PASS | browser/components/urlbar/tests/quicksuggest/browser/browser_quicksuggest_ping_amp.js | The actual ping should have key: pingType - true == true - 
[task 2025-02-03T13:50:42.734Z] 13:50:42     INFO - Buffered messages finished
[task 2025-02-03T13:50:42.734Z] 13:50:42     INFO - TEST-UNEXPECTED-FAIL | browser/components/urlbar/tests/quicksuggest/browser/browser_quicksuggest_ping_amp.js | The actual ping should have the correct value for key: pingType - null === "quicksuggest-impression" - 
[task 2025-02-03T13:50:42.734Z] 13:50:42     INFO - Stack trace:
[task 2025-02-03T13:50:42.734Z] 13:50:42     INFO - chrome://mochitests/content/browser/browser/components/urlbar/tests/quicksuggest/browser/head.js:assertQuickSuggestPing:624
[task 2025-02-03T13:50:42.734Z] 13:50:42     INFO - chrome://mochitests/content/browser/browser/components/urlbar/tests/quicksuggest/browser/head.js:checkPing:545
[task 2025-02-03T13:50:42.734Z] 13:50:42     INFO - resource:///modules/urlbar/private/AmpSuggestions.sys.mjs:#submitQuickSuggestPing:303
[task 2025-02-03T13:50:42.734Z] 13:50:42     INFO - resource:///modules/urlbar/private/AmpSuggestions.sys.mjs:#submitQuickSuggestImpressionPing:307
[task 2025-02-03T13:50:42.734Z] 13:50:42     INFO - resource:///modules/urlbar/private/AmpSuggestions.sys.mjs:onImpression:183
[task 2025-02-03T13:50:42.734Z] 13:50:42     INFO - resource:///modules/UrlbarProviderQuickSuggest.sys.mjs:onImpression:254
[task 2025-02-03T13:50:42.734Z] 13:50:42     INFO - resource:///modules/UrlbarUtils.sys.mjs:tryMethod:2437
[task 2025-02-03T13:50:42.735Z] 13:50:42     INFO - resource:///modules/UrlbarProvidersManager.sys.mjs:#notifyImpression:451
[task 2025-02-03T13:50:42.735Z] 13:50:42     INFO - resource:///modules/UrlbarProvidersManager.sys.mjs:notifyEngagementChange:391
[task 2025-02-03T13:50:42.735Z] 13:50:42     INFO - resource:///modules/UrlbarController.sys.mjs:#internalRecord:1062
[task 2025-02-03T13:50:42.735Z] 13:50:42     INFO - resource:///modules/UrlbarController.sys.mjs:record:925
[task 2025-02-03T13:50:42.735Z] 13:50:42     INFO - resource:///modules/UrlbarInput.sys.mjs:pickResult:1400
[task 2025-02-03T13:50:42.735Z] 13:50:42     INFO - resource:///modules/UrlbarInput.sys.mjs:pickElement:998
[task 2025-02-03T13:50:42.735Z] 13:50:42     INFO - resource:///modules/UrlbarView.sys.mjs:on_mouseup:3522
[task 2025-02-03T13:50:42.735Z] 13:50:42     INFO - resource:///modules/UrlbarView.sys.mjs:handleEvent:974
[task 2025-02-03T13:50:42.735Z] 13:50:42     INFO - chrome://mochikit/content/tests/SimpleTest/EventUtils.js:synthesizeMouseAtPoint:825
[task 2025-02-03T13:50:42.735Z] 13:50:42     INFO - chrome://mochikit/content/tests/SimpleTest/EventUtils.js:synthesizeMouse:557
[task 2025-02-03T13:50:42.735Z] 13:50:42     INFO - chrome://mochikit/content/tests/SimpleTest/EventUtils.js:synthesizeMouseAtCenter:989
[task 2025-02-03T13:50:42.735Z] 13:50:42     INFO - chrome://mochitests/content/browser/browser/components/urlbar/tests/quicksuggest/browser/head.js:doImpressionOnlyTest/<:293
[task 2025-02-03T13:50:42.735Z] 13:50:42     INFO - resource://testing-common/UrlbarTestUtils.sys.mjs:promisePopupClose:755
[task 2025-02-03T13:50:42.735Z] 13:50:42     INFO - chrome://mochitests/content/browser/browser/components/urlbar/tests/quicksuggest/browser/head.js:doImpressionOnlyTest:292
[task 2025-02-03T13:50:42.736Z] 13:50:42     INFO - chrome://mochitests/content/browser/browser/components/urlbar/tests/quicksuggest/browser/head.js:doQuickSuggestPingTest:178
[task 2025-02-03T13:50:42.736Z] 13:50:42     INFO - chrome://mochitests/content/browser/browser/components/urlbar/tests/quicksuggest/browser/browser_quicksuggest_ping_amp.js:amp:43
[task 2025-02-03T13:50:42.736Z] 13:50:42     INFO - chrome://mochikit/content/browser-test.js:handleTask:1147
[task 2025-02-03T13:50:42.736Z] 13:50:42     INFO - chrome://mochikit/content/browser-test.js:_runTaskBasedTest:1219
[task 2025-02-03T13:50:42.736Z] 13:50:42     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1360
[task 2025-02-03T13:50:42.736Z] 13:50:42     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1136
[task 2025-02-03T13:50:42.736Z] 13:50:42     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/<:1058
[task 2025-02-03

this check appears to be failing, but I don't know why. These are also metric check failures so it's possible that they are related to the failures in bug 1775795 and/or bug 1945662.

Comment 1

[Drew Willcoxon :adw]

Bug 1943977 is part of the fix, but this test still fails for me on a local artifact build. From what I debugged, Glean core still needs a fix too (see comment 0 in that bug). Jan-Erik or chutten, could you comment on remaining work please?

Comment 2

[Jan-Erik Rediger [:janerik]]

I'll take a look. I might indeed have missed something.

Comment 3

[Jan-Erik Rediger [:janerik]]

Well, at least I can still reproduce it locally now.

Comment 4

[Jan-Erik Rediger [:janerik]]

Alright. The issue here indeed is that setEnabled is still called on the compiled-in ping, not on the JOG registered one. I don't know exactly yet why the code doesn't always use the JOG-provided one, nor do I have a good plan how to more generally prevent this.
But I'm working on it.

Comment 5

[Jan-Erik Rediger [:janerik]]

Hah, with a little bit more time I can now say: setEnabled is called on the JOG ping. But later the compiled-in pings are (re-)registered, thus overwriting the previous JOG pings.

This is not as easy to solve.
We're dealing with mutable global state, spread across separate parts of the code base, that gets invoked in a non-deterministic order.
JOG metrics are registered on first access to them through GleanPings.
Compiled-in pings are registered when FOG is initialized. That might happen after the first GleanPings access.
On top of that in theory it's possible to hold on to a GleanPings ping, reset Glean in tests and re-access the previously held ping, which is already overriden by re-applying JOG in the reset.

What we would really need is a single consistent global registry of pings, so for every ping name every part of the code agrees which instance we're talking about.
I'll sync with chutten what we gonna do about it.

Side: This is not an issue for metrics, because they don't carry much state around. The bit of state we have (enabled) is rarely changed and on top we track its config ("server knobs") using epochs (so re-applying a server-knobs config will make sure every metric will refetch its status from that config).

Comment 6

[Jan-Erik Rediger [:janerik]]

Attachment: Bug 1945663 - Properly reset and reload JOG after test reset r?chutten!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/59cd341dbf47
Properly reset and reload JOG after test reset r=chutten

Comment 8

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/59cd341dbf47