1001039 - [e10s] More reftest changes

Status: RESOLVED FIXED

(Testing :: Reftest, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: apzc-reftest-fixes

This first patch changes the annotations on the APZC tests. Previously they assumed that browserIsRemote is true iff APZC is enabled. This is no longer true. I added a new asyncPanZoom test that you can now use in the reftest manifests as a substitute.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: ignore-plugin-tests

The test plugin doesn't get loaded properly in e10s right now. This test ensures that tests requiring the test plugin don't get run in e10s.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: disable-spellcheck-tests

Spellchecking is also broken in e10s right now. This patch disables those tests.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8412019 [details] [diff] [review]
disable-spellcheck-tests

Review of attachment 8412019 [details] [diff] [review]:
-----------------------------------------------------------------

Can we set layout.spellcheckDefault to 0 for e10s?

I think we should have a "spellcheckEnabled" reftest variable, use that here, and set it based on the pref or e10s.

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: disable-spellcheck-tests v2

How does this look?

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8412208 [details] [diff] [review]
disable-spellcheck-tests v2

Review of attachment 8412208 [details] [diff] [review]:
-----------------------------------------------------------------

::: editor/reftests/reftest.list
@@ +16,5 @@
>  needs-focus == passwd-4.html passwd-ref.html
>  == emptypasswd-1.html emptypasswd-ref.html
>  == emptypasswd-2.html emptypasswd-ref.html
>  == caret_on_positioned.html caret_on_positioned-ref.html
> +skip-if(spellcheckEnabled) skip-if(B2G) fails-if(Android) needs-focus != spellcheck-input-disabled.html spellcheck-input-ref.html

Shouldn't this be skip-if(!spellcheckEnabled)? And I don't think you need the B2G/Android annotations anymore.

::: layout/tools/reftest/reftest.js
@@ +723,5 @@
>          sandbox.asyncPanZoom = false;
>      }
>  
> +    try {
> +        sandbox.spellcheckEnabled = prefs.getBoolPref("layout.spellcheckDefault") && !gBrowserIsRemote;

It's an intPref. And you should be checking it's != 0.

@@ +725,5 @@
>  
> +    try {
> +        sandbox.spellcheckEnabled = prefs.getBoolPref("layout.spellcheckDefault") && !gBrowserIsRemote;
> +    } catch (e) {
> +        sandbox.spellcheckDefault = false;

sandbox.spellcheckEnabled = false;

Comment 6

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: disable-spellcheck-tests v3

Sorry about the bugs in the previous patch. This removes all the Android/B2G annotations. This change does have the effect of converting some fail-if/random-if annotations to skip-if. I don't think this makes much difference, but I thought I should point it out.

Comment 7

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I didn't land the spellchecking changes since we're pretty close to getting that working.

https://hg.mozilla.org/integration/mozilla-inbound/rev/7a31c8d90a6c
https://hg.mozilla.org/integration/mozilla-inbound/rev/f5a00089d1c0

Comment 8

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

A test called Rs-oop, which hadn't been running the APZ scrolling tests, started running and failing them because of this patch. Strangely, given the name, these tests run in-process! So I put the !browserIsRemote condition back so that we don't fail. Something needs to be done about this though. Does it make sense that these run in-process Rob?

https://hg.mozilla.org/integration/mozilla-inbound/rev/6574f66e8d47

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I don't know anything about Rs-oop but this change looks OK.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7a31c8d90a6c
https://hg.mozilla.org/mozilla-central/rev/f5a00089d1c0
https://hg.mozilla.org/mozilla-central/rev/6574f66e8d47