Closed Bug 1635806 Opened 4 years ago Closed 4 years ago

BrowserTestUtils.waitForContentEvent returning eventName

Categories

(Testing :: Mochitest, enhancement, P3)

Product:
Testing
Component:
Mochitest
Type:
enhancement

Tracking

(firefox78 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla78
Tracking Flags:
Tracking Status
firefox78 --- fixed

People

(Reporter: etienne, Assigned: etienne)

Details

Attachments

(1 file)

Bug 1635806 - [testing] Expose eventName in BrowserTestUtils.waitForContentEvent r?ahal
47 bytes, text/x-phabricator-request
Details | Review

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/84.0.4128.3 Safari/537.36

Steps to reproduce:

Looking at this example usage of the BrowserTestUtils.waitForContentEvent-method:

  const offlineChanged = Promise.race([
    BrowserTestUtils.waitForContentEvent(gBrowser.selectedBrowser, "online", true).then(() => "online"),
    BrowserTestUtils.waitForContentEvent(gBrowser.selectedBrowser, "offline", true).then(() => "offline"),
  ])

Actual results:

I had to add .then(() => "online") to figure out which event was first.

Expected results:

The BrowserTestUtils.waitForContentEvent-Promise should at least return the eventName upon resolving, instead of just undefined.

This would make testing for race-conditions easier.

Compared to waitForEvent we cannot return the whole event here because it's a cyclic object. It would be good to know which other properties might be helpful to send over here. The event name here will be event.type.

Andrew, any feedback from your side? Otherwise Etienne would be happy to provide a fix for that bug.

Flags: needinfo?(ahal)

Nope, no special feedback. Your suggestion sounds fine to me.

Flags: needinfo?(ahal)
Priority: -- → P3

This should make it easier to write tests for race-conditions, as
the resolving value now indicates which event was received.

Assignee: nobody → etienne
Status: UNCONFIRMED → NEW
Ever confirmed: true
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ea9641bad8d6
[testing] Expose eventName in BrowserTestUtils.waitForContentEvent r=ahal

Backed out changeset ea9641bad8d6 (bug 1635806) for browser_fullscreen-contextmenu-esc.js failures

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=mochitest&fromchange=ea9641bad8d64f59c4e8f2dba7b2b4a553fa4e41&tochange=efb86c809f5c1bba866e1e8585dab7341bb9828a&selectedTaskRun=Aqn2qYdsRlK9-UPJ5ZsHxA-0

Backout link: https://hg.mozilla.org/integration/autoland/rev/efb86c809f5c1bba866e1e8585dab7341bb9828a

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=301523048&repo=autoland&lineNumber=15813

[task 2020-05-09T06:28:31.979Z] 06:28:31     INFO - TEST-START | dom/html/test/browser_fullscreen-contextmenu-esc.js
[task 2020-05-09T06:28:32.039Z] 06:28:32     INFO - GECKO(5158) | [Child 5367: Main Thread]: I/DocShellAndDOMWindowLeak ++DOCSHELL 0x7fc1e8fa0c00 == 2 [pid = 5367] [id = {84f46d5d-0f8f-4095-9e39-3f7e7964f2b6}]
[task 2020-05-09T06:28:32.042Z] 06:28:32     INFO - GECKO(5158) | [Child 5367: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 5 (0x7fc1e8f28090) [pid = 5367] [serial = 28] [outer = (nil)]
[task 2020-05-09T06:28:32.043Z] 06:28:32     INFO - GECKO(5158) | [Child 5367: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 6 (0x7fc1e8fa2800) [pid = 5367] [serial = 29] [outer = 0x7fc1e8f28090]
[task 2020-05-09T06:28:32.078Z] 06:28:32     INFO - GECKO(5158) | Waiting for browser load
[task 2020-05-09T06:28:32.119Z] 06:28:32     INFO - GECKO(5158) | Saw state f0001 and status 0
[task 2020-05-09T06:28:32.201Z] 06:28:32     INFO - GECKO(5158) | [Child 5367: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 7 (0x7fc1e8fa8c00) [pid = 5367] [serial = 30] [outer = 0x7fc1e8f28090]
[task 2020-05-09T06:28:32.297Z] 06:28:32     INFO - GECKO(5158) | Saw state c0010 and status 0
[task 2020-05-09T06:28:32.297Z] 06:28:32     INFO - GECKO(5158) | Browser loaded http://example.org/browser/dom/html/test/dummy_page.html
[task 2020-05-09T06:28:32.985Z] 06:28:32     INFO - GECKO(5158) | [Child 5494: Main Thread]: I/DocShellAndDOMWindowLeak --DOCSHELL 0x7f98c08da000 == 0 [pid = 5494] [id = {0c21da14-37ae-4147-b77d-247ac3a79bbb}] [url = about:blank]
[task 2020-05-09T06:28:33.001Z] 06:28:33     INFO - GECKO(5158) | [Child 5284: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 8 (0x7f5c679d3400) [pid = 5284] [serial = 23] [outer = (nil)] [url = about:blank]
[task 2020-05-09T06:28:33.001Z] 06:28:33     INFO - GECKO(5158) | [Child 5284: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 7 (0x7f5c679d4000) [pid = 5284] [serial = 21] [outer = (nil)] [url = about:blank]
[task 2020-05-09T06:28:33.002Z] 06:28:33     INFO - GECKO(5158) | [Child 5284: Main Thread]: I/DocShellAndDOMWindowLeak --DOCSHELL 0x7f5c679d1000 == 2 [pid = 5284] [id = {0ee4300e-e6b2-4f24-934d-a240b686e151}] [url = about:blank]
[task 2020-05-09T06:28:33.002Z] 06:28:33     INFO - GECKO(5158) | [Child 5284: Main Thread]: I/DocShellAndDOMWindowLeak --DOCSHELL 0x7f5c679d0400 == 1 [pid = 5284] [id = {f76b1384-b8d6-45d1-9ce6-c5c0d7b676be}] [url = http://example.org/browser/dom/html/test/file_fullscreen-api-keys.html]
[task 2020-05-09T06:28:33.057Z] 06:28:33     INFO - GECKO(5158) | [Child 5494: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 1 (0x7f98d6093350) [pid = 5494] [serial = 1] [outer = (nil)] [url = about:blank]
[task 2020-05-09T06:28:33.832Z] 06:28:33     INFO - GECKO(5158) | [Parent 5158: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 22 (0x7fd4a5638800) [pid = 5158] [serial = 17] [outer = (nil)] [url = about:blank]
[task 2020-05-09T06:28:33.833Z] 06:28:33     INFO - GECKO(5158) | [Parent 5158: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 21 (0x7fd4a3cb7800) [pid = 5158] [serial = 21] [outer = (nil)] [url = about:blank]
[task 2020-05-09T06:28:33.834Z] 06:28:33     INFO - GECKO(5158) | [Parent 5158: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 20 (0x7fd4abe0b400) [pid = 5158] [serial = 25] [outer = (nil)] [url = about:blank]
[task 2020-05-09T06:28:33.834Z] 06:28:33     INFO - GECKO(5158) | [Parent 5158, Main Thread] WARNING: Wrong button set to eContextMenu event?: 'mMessage != eContextMenu || mButton == ((mContextMenuTrigger == eNormal) ? MouseButton::eRight : MouseButton::eLeft)', file /builds/worker/workspace/obj-build/dist/include/mozilla/MouseEvents.h, line 238
[task 2020-05-09T06:28:34.234Z] 06:28:34     INFO - GECKO(5158) | [Parent 5158: Main Thread]: I/DocShellAndDOMWindowLeak --DOMWINDOW == 19 (0x7fd4a9c90180) [pid = 5158] [serial = 28] [outer = (nil)] [url = about:blank]
[task 2020-05-09T06:28:34.478Z] 06:28:34     INFO - TEST-INFO | started process screentopng
[task 2020-05-09T06:28:34.829Z] 06:28:34     INFO - TEST-INFO | screentopng: exit 0
[task 2020-05-09T06:28:34.830Z] 06:28:34     INFO - Buffered messages logged at 06:28:31
[task 2020-05-09T06:28:34.831Z] 06:28:34     INFO - Entering test bound 
[task 2020-05-09T06:28:34.832Z] 06:28:34     INFO - Buffered messages logged at 06:28:32
[task 2020-05-09T06:28:34.832Z] 06:28:34     INFO - TEST-PASS | dom/html/test/browser_fullscreen-contextmenu-esc.js | Got context menu - 
[task 2020-05-09T06:28:34.833Z] 06:28:34     INFO - Enter DOM fullscreen
[task 2020-05-09T06:28:34.834Z] 06:28:34     INFO - TEST-PASS | dom/html/test/browser_fullscreen-contextmenu-esc.js | The content should have entered fullscreen - 
[task 2020-05-09T06:28:34.835Z] 06:28:34     INFO - TEST-PASS | dom/html/test/browser_fullscreen-contextmenu-esc.js | The chrome should also be in fullscreen - 
[task 2020-05-09T06:28:34.836Z] 06:28:34     INFO - Open context menu
[task 2020-05-09T06:28:34.836Z] 06:28:34     INFO - TEST-PASS | dom/html/test/browser_fullscreen-contextmenu-esc.js | Should not have opened context menu - 
[task 2020-05-09T06:28:34.838Z] 06:28:34     INFO - Waiting for popupshown
[task 2020-05-09T06:28:34.839Z] 06:28:34     INFO - Buffered messages logged at 06:28:33
[task 2020-05-09T06:28:34.840Z] 06:28:34     INFO - Saw popupshown
[task 2020-05-09T06:28:34.840Z] 06:28:34     INFO - TEST-PASS | dom/html/test/browser_fullscreen-contextmenu-esc.js | Should have opened context menu - 
[task 2020-05-09T06:28:34.841Z] 06:28:34     INFO - Send the first escape
[task 2020-05-09T06:28:34.842Z] 06:28:34     INFO - Waiting for popuphidden
[task 2020-05-09T06:28:34.843Z] 06:28:34     INFO - Saw popuphidden
[task 2020-05-09T06:28:34.843Z] 06:28:34     INFO - TEST-PASS | dom/html/test/browser_fullscreen-contextmenu-esc.js | Should have closed context menu - 
[task 2020-05-09T06:28:34.844Z] 06:28:34     INFO - Buffered messages logged at 06:28:34
[task 2020-05-09T06:28:34.845Z] 06:28:34     INFO - TEST-PASS | dom/html/test/browser_fullscreen-contextmenu-esc.js | The content should still be in fullscreen - 
[task 2020-05-09T06:28:34.846Z] 06:28:34     INFO - TEST-PASS | dom/html/test/browser_fullscreen-contextmenu-esc.js | The chrome should still be in fullscreen - 
[task 2020-05-09T06:28:34.846Z] 06:28:34     INFO - Send the second escape
[task 2020-05-09T06:28:34.847Z] 06:28:34     INFO - Buffered messages finished
[task 2020-05-09T06:28:34.851Z] 06:28:34     INFO - TEST-UNEXPECTED-FAIL | dom/html/test/browser_fullscreen-contextmenu-esc.js | The content should have exited fullscreen - 
[task 2020-05-09T06:28:34.852Z] 06:28:34     INFO - Stack trace:
[task 2020-05-09T06:28:34.852Z] 06:28:34     INFO - chrome://mochikit/content/browser-test.js:test_ok:1299
[task 2020-05-09T06:28:34.853Z] 06:28:34     INFO - chrome://mochitests/content/browser/dom/html/test/browser_fullscreen-contextmenu-esc.js:null:110
[task 2020-05-09T06:28:34.854Z] 06:28:34     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1064
[task 2020-05-09T06:28:34.855Z] 06:28:34     INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1104
[task 2020-05-09T06:28:34.856Z] 06:28:34     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:927
[task 2020-05-09T06:28:34.856Z] 06:28:34     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:918
[task 2020-05-09T06:28:34.857Z] 06:28:34     INFO - TEST-PASS | dom/html/test/browser_fullscreen-contextmenu-esc.js | The chrome should have exited fullscreen - 
[task 2020-05-09T06:28:34.858Z] 06:28:34     INFO - Leaving test bound 
[task 2020-05-09T06:28:34.859Z] 06:28:34     INFO - GECKO(5158) | [Child 5367, Main Thread] WARNING: NS_ENSURE_TRUE(mDoneSetup) failed: file /builds/worker/checkouts/gecko/editor/composer/nsEditingSession.cpp, line 1185
[task 2020-05-09T06:28:34.860Z] 06:28:34     INFO - GECKO(5158) | JavaScript error: resource://gre/modules/URIFixup.jsm, line 207: NS_ERROR_FAILURE: Should pass a non-null uri
[task 2020-05-09T06:28:34.863Z] 06:28:34     INFO - Console message: [JavaScript Error: "NS_ERROR_FAILURE: Should pass a non-null uri" {file: "resource://gre/modules/URIFixup.jsm" line: 207}]
[task 2020-05-09T06:28:34.864Z] 06:28:34     INFO - getFixupURIInfo@resource://gre/modules/URIFixup.jsm:207:13
[task 2020-05-09T06:28:34.865Z] 06:28:34     INFO - _getUrlMetaData@resource:///modules/UrlbarValueFormatter.jsm:131:35
[task 2020-05-09T06:28:34.866Z] 06:28:34     INFO - _ensureFormattedHostVisible/<@resource:///modules/UrlbarValueFormatter.jsm:86:41
[task 2020-05-09T06:28:34.867Z] 06:28:34     INFO - FrameRequestCallback*_ensureFormattedHostVisible@resource:///modules/UrlbarValueFormatter.jsm:76:17
[task 2020-05-09T06:28:34.867Z] 06:28:34     INFO - _on_resize/this._resizeThrottleTimeout<@resource:///modules/UrlbarValueFormatter.jsm:466:12
[task 2020-05-09T06:28:34.868Z] 06:28:34     INFO - setTimeout handler*SimpleTest_setTimeoutShim@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:788:41
[task 2020-05-09T06:28:34.869Z] 06:28:34     INFO - _on_resize@resource:///modules/UrlbarValueFormatter.jsm:464:47
[task 2020-05-09T06:28:34.870Z] 06:28:34     INFO - handleEvent@resource:///modules/UrlbarValueFormatter.jsm:448:23
[task 2020-05-09T06:28:34.871Z] 06:28:34     INFO - EventListener.handleEvent*UrlbarValueFormatter@resource:///modules/UrlbarValueFormatter.jsm:38:17
[task 2020-05-09T06:28:34.872Z] 06:28:34     INFO - UrlbarInput/<@resource:///modules/UrlbarInput.jsm:156:14
[task 2020-05-09T06:28:34.873Z] 06:28:34     INFO - get@resource://gre/modules/XPCOMUtils.jsm:129:51
[task 2020-05-09T06:28:34.874Z] 06:28:34     INFO - formatValue@resource:///modules/UrlbarInput.jsm:230:7
[task 2020-05-09T06:28:34.875Z] 06:28:34     INFO - onSecurityChange@chrome://browser/content/browser.js:5429:13
[task 2020-05-09T06:28:34.875Z] 06:28:34     INFO - callListeners@chrome://browser/content/tabbrowser.js:811:31
[task 2020-05-09T06:28:34.876Z] 06:28:34     INFO - _callProgressListeners@chrome://browser/content/tabbrowser.js:825:22
[task 2020-05-09T06:28:34.877Z] 06:28:34     INFO - _callProgressListeners@chrome://browser/content/tabbrowser.js:5782:46
[task 2020-05-09T06:28:34.878Z] 06:28:34     INFO - onSecurityChange@chrome://browser/content/tabbrowser.js:6240:12
[task 2020-05-09T06:28:34.880Z] 06:28:34     INFO - _callProgressListeners@resource://gre/modules/RemoteWebProgress.jsm:75:31
[task 2020-05-09T06:28:34.881Z] 06:28:34     INFO - onSecurityChange@resource://gre/modules/RemoteWebProgress.jsm:141:10
[task 2020-05-09T06:28:34.882Z] 06:28:34     INFO - 
[task 2020-05-09T06:28:34.883Z] 06:28:34     INFO - GECKO(5158) | MEMORY STAT | vsize 3385MB | residentFast 414MB | heapAllocated 141MB
[task 2020-05-09T06:28:34.884Z] 06:28:34     INFO - TEST-OK | dom/html/test/browser_fullscreen-contextmenu-esc.js | took 2650ms
Flags: needinfo?(etienne)

Etienne, you should be able to reproduce the problem pretty easily by running the following command with your patch attached:

./mach test dom/html/test/browser_fullscreen-contextmenu-esc.js

Then you will see that the problematic lines are:

https://searchfox.org/mozilla-central/rev/fa52bedc4b401c12251513fa1c9df1753a29abb2/dom/html/test/browser_fullscreen-contextmenu-esc.js#109-110

Here the returned value is checked for undefined, which is not true anymore. I would suggest to introduce a event variable, and compare it with fullscreenchange. And similar for the fullScreenChangedPromise promise above.

Status: NEW → ASSIGNED

The check that was in place, did not serve any purpose. The event was already awaited, and therefore the resolving value did not need assertion. It either resolves, rejects or gives a timeout. Both timeout and rejection automatically fail the test - which means resolving is the only course of action that can happen after the await.

Care to take a look at the updated patch? That should solve the issue, while retaining the amount of testability as before this patch.

Flags: needinfo?(etienne) → needinfo?(hskupin)
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/74c47aff0a29
[testing] Expose eventName in BrowserTestUtils.waitForContentEvent r=ahal

https://hg.mozilla.org/mozilla-central/rev/74c47aff0a29

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox78: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
Flags: needinfo?(hskupin)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: