Closed Bug 1001039 Opened 10 years ago Closed 9 years ago

[e10s] More reftest changes


(Testing :: Reftest, defect)

Not set



Tracking Status
e10s + ---


(Reporter: billm, Assigned: billm)


(Blocks 1 open bug)



(3 files, 2 obsolete files)

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.
Attachment #8412017 - Flags: review?(roc)
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.
Attachment #8412018 - Flags: review?(roc)
Attached patch disable-spellcheck-tests (obsolete) — Splinter Review
Spellchecking is also broken in e10s right now. This patch disables those tests.
Assignee: nobody → wmccloskey
Attachment #8412019 - Flags: review?(roc)
Comment on attachment 8412019 [details] [diff] [review]

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.
Attachment #8412019 - Flags: review?(roc) → review-
Attached patch disable-spellcheck-tests v2 (obsolete) — Splinter Review
How does this look?
Attachment #8412019 - Attachment is obsolete: true
Attachment #8412208 - Flags: review?(roc)
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;
Attachment #8412208 - Flags: review?(roc) → review-
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.
Attachment #8412208 - Attachment is obsolete: true
Attachment #8413914 - Flags: review?(roc)
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?
Flags: needinfo?(roc)
I don't know anything about Rs-oop but this change looks OK.
Flags: needinfo?(roc)
You need to log in before you can comment on or make changes to this bug.