Closed Bug 1311197 Opened 5 years ago Closed 5 years ago

Remove more CPOWs from tests

Categories

(Firefox :: General, defect)

Product:
Firefox
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 52
Tracking Flags:
Tracking Status
firefox51 --- fixed
firefox52 --- fixed

People

(Reporter: mrbkap, Assigned: mrbkap)

Details

Attachments

(7 files)

Bug 1311197 - Remove CPOWs from browser_bug611242.js.
58 bytes, text/x-review-board-request
Gijs
: review+
Details
Bug 1311197 - Remove CPOWs from browser_localfile2.js.
58 bytes, text/x-review-board-request
Gijs
: review+
Details
Bug 1311197 - Remove CPOWs from browser_bug655270.js.
58 bytes, text/x-review-board-request
Gijs
: review+
Details
Bug 1311197 - Remove CPOWs from some plugin mochitests.
58 bytes, text/x-review-board-request
Gijs
: review+
Details
Bug 1311197 - Remove CPOWs from urlbar tests.
58 bytes, text/x-review-board-request
Gijs
: review+
Details
Bug 1311197 - Remove CPOWs from docshell browser-chrome tests.
58 bytes, text/x-review-board-request
Gijs
: review+
Details
Bug 1311197 - Remove a shim use from devtools tests.
58 bytes, text/x-review-board-request
Gijs
: review+
Details 
I wrote a script to extract CPOW uses in test files[1]. I fixed a bunch of them and will try to get some help for the rest as well as fixing more when I have time.

[1] https://docs.google.com/document/d/1EUUi3tMuhHsJA9iE9yNvjWcae4Unmqrl4uN2VwYxIqE/edit
Attachment #8802316 - Flags: review?(felipc) → review?(gijskruitbosch+bugs)
Attachment #8802317 - Flags: review?(felipc) → review?(gijskruitbosch+bugs)
Attachment #8802318 - Flags: review?(felipc) → review?(gijskruitbosch+bugs)
Attachment #8802319 - Flags: review?(felipc) → review?(gijskruitbosch+bugs)
Attachment #8802320 - Flags: review?(felipc) → review?(gijskruitbosch+bugs)
Comment on attachment 8802319 [details]
Bug 1311197 - Remove CPOWs from some plugin mochitests.

https://reviewboard.mozilla.org/r/86732/#review85846

::: browser/base/content/test/plugins/browser_bug743421.js:92
(Diff revision 1)
>      let plugin = content.document.getElementsByTagName("embed")[1];
>      let objLoadingContent = plugin.QueryInterface(Ci.nsIObjectLoadingContent);
>      Assert.ok(objLoadingContent.activated, "Test 1d, Plugin should be activated");
> -  });
>  
> -  let promise = waitForEvent(gTestBrowser.contentWindow, "hashchange", null);
> +    let promise = ContentTaskUtils.waitForEvent(content, "hashchange")

Nit: missing semicolon.
Attachment #8802319 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8802320 [details]
Bug 1311197 - Remove CPOWs from urlbar tests.

https://reviewboard.mozilla.org/r/86734/#review85854
Attachment #8802320 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8802316 [details]
Bug 1311197 - Remove CPOWs from browser_bug611242.js.

https://reviewboard.mozilla.org/r/86726/#review85858
Attachment #8802316 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8802317 [details]
Bug 1311197 - Remove CPOWs from browser_localfile2.js.

https://reviewboard.mozilla.org/r/86728/#review85860
Attachment #8802317 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8802318 [details]
Bug 1311197 - Remove CPOWs from browser_bug655270.js.

https://reviewboard.mozilla.org/r/86730/#review85862
Attachment #8802318 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8802321 [details]
Bug 1311197 - Remove CPOWs from docshell browser-chrome tests.

https://reviewboard.mozilla.org/r/86736/#review85864

::: docshell/test/browser/browser_tab_touch_events.js
(Diff revision 1)
> -  });
> -}
> -
> -function openUrl(url) {
> -  return new Promise(function(resolve, reject) {
> -    window.focus();

To keep this the same, behaviour-wise, I think we'd have to stick the window.focus() inside the add_task function?

I'm not sure why this test would need it, but it feels like removing this could potentially change behaviour, either for this or following tests. OTOH, perhaps we should find out rather than having undocumented 'magic' lying around in the test?

(not stealing this review because maybe Felipe knows more here - otherwise, r=me with the window.focus() call kept or an explanation why it's useless)
Attachment #8802321 - Flags: review+
Attachment #8802322 - Flags: review?(felipc) → review?(gijskruitbosch+bugs)
Comment on attachment 8802322 [details]
Bug 1311197 - Remove a shim use from devtools tests.

https://reviewboard.mozilla.org/r/86738/#review85866
Attachment #8802322 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8802321 [details]
Bug 1311197 - Remove CPOWs from docshell browser-chrome tests.

https://reviewboard.mozilla.org/r/86736/#review86300
Attachment #8802321 - Flags: review?(felipc) → review+
Attachment #8802319 - Flags: review?(felipc)
Pushed by mrbkap@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9ce4a72010fa
Remove CPOWs from browser_bug611242.js. r=Gijs
https://hg.mozilla.org/integration/mozilla-inbound/rev/fbd597046674
Remove CPOWs from browser_localfile2.js. r=Gijs
https://hg.mozilla.org/integration/mozilla-inbound/rev/d30948200f18
Remove CPOWs from browser_bug655270.js. r=Gijs
https://hg.mozilla.org/integration/mozilla-inbound/rev/eb3c939c1c22
Remove CPOWs from some plugin mochitests. r=Gijs
https://hg.mozilla.org/integration/mozilla-inbound/rev/144e317c6fb2
Remove CPOWs from urlbar tests. r=Gijs
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9d180c51943
Remove CPOWs from docshell browser-chrome tests. r=Gijs/felipe
https://hg.mozilla.org/integration/mozilla-inbound/rev/8e998fdc243d
Remove a shim use from devtools tests. r=Gijs
You need to log in before you can comment on or make changes to this bug.