Still a race condition in devtools/client/aboutdebugging-new/test/browser/browser_aboutdebugging_telemetry_runtime_updates.js
Categories
(DevTools :: about:debugging, enhancement)
Tracking
(firefox67 fixed)
Tracking | Status | |
---|---|---|
firefox67 | --- | fixed |
People
(Reporter: jorendorff, Assigned: jorendorff)
References
Details
Attachments
(1 file)
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.)
Assignee | ||
Comment 1•6 years ago
|
||
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);
checkTelemetryEvents([
{ method: "runtime_disconnected", extras: runtime1ConnectedExtras },
{ method: "runtime_removed", extras: runtime1Extras },
], sessionId);
info("Remove runtime 2");
await removeUsbRuntime(USB_RUNTIME_2, mocks, document);
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 2•6 years ago
|
||
Assignee | ||
Comment 3•6 years ago
|
||
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 | ||
Comment 4•6 years ago
|
||
Assignee | ||
Updated•6 years ago
|
Comment 6•6 years ago
|
||
bugherder |
Updated•6 years ago
|
Description
•