Still a race condition in devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_telemetry_runtime_updates.js

RESOLVED FIXED in Firefox 67


5 months ago
4 months ago


(Reporter: jorendorff, Assigned: jorendorff)


Firefox 67
Dependency tree / graph

Firefox Tracking Flags

(firefox67 fixed)



(1 attachment)

In bug 1495072, I'm changing await to be about 2x faster (each await currently creates an extra Promise, for no good reason -- it's redundant -- and the change is to stop doing that).

This breaks some tests, including this one.

The issue seems to be that the UI updates (and therefore removeUsbRuntime thinks it's done) before all Telemetry events have been recorded; making await faster alters the order of events enough to make the race condition bite.

(Note: This was previously reported in bug 1530833, and a patch was landed there, but that doesn't fix the issue for me.)

This patch makes it pass again, for me locally, but—well, it's not a very good fix, obviously...

diff --git a/devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_telemetry_runtime_updates.js b/dev
--- a/devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_telemetry_runtime_updates.js
+++ b/devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_telemetry_runtime_updates.js
@@ -89,16 +89,17 @@ add_task(async function testUsbRuntimeUp
   // for all the events related to RUNTIME_2.
   const runtime2Id = evts.filter(e => e.method === "runtime_added")[0].extras.runtime_id;
   const runtime2Extras = Object.assign({}, RUNTIME_2_EXTRAS, {
     "runtime_id": runtime2Id,
   info("Remove runtime 1");
   await removeUsbRuntime(USB_RUNTIME_1, mocks, document);
+  await Promise.resolve(0);
     { method: "runtime_disconnected", extras: runtime1ConnectedExtras },
     { method: "runtime_removed", extras: runtime1Extras },
   ], sessionId);
   info("Remove runtime 2");
   await removeUsbRuntime(USB_RUNTIME_2, mocks, document);
Flags: needinfo?(balbeza)

I really want to land the new faster await for Firefox 67, so I'm going to post a patch here, along the lines of comment 1, to work around this for the time being. That's what we did in bug 1525395, for another devtools test that was waiting for the UI to update.

I wish I knew a better way to do it, but a just slightly quirky test shouldn't hold up this big performance win...

Assignee: nobody → jorendorff
Blocks: 1495072
Pushed by
Work around a race condition in an about:debugging test. r=jlast
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67
Flags: needinfo?(balbeza)
You need to log in before you can comment on or make changes to this bug.