1093756 - re-enable several tests for e10s

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[:Gijs (he/him)]

2 INFO TEST-START | chrome://mochitests/content/browser/browser/base/content/test/general/browser_bug581253.js
3 INFO Console message: [JavaScript Error: "The character encoding of the plain text document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the file needs to be declared in the transfer protocol or file needs to use a byte order mark as an encoding signature." {file: "data:text/plain,nothing%20but%20plain%20text" line: 0}]
4 INFO TEST-PASS | chrome://mochitests/content/browser/browser/base/content/test/general/browser_bug581253.js | the test url is bookmarked 
5 INFO Waiting for star button change.
6 INFO Waiting for star button change.
7 INFO Waiting for star button change.
8 INFO Waiting for star button change.
9 INFO Waiting for star button change.
10 INFO Waiting for star button change.

(... continues indefinitely until timeout ...)

Interestingly, even manually clicking the star button (a) doesn't seem to successfully bookmark the page, and (b) doesn't stop the spam

Comment 1

[Tracy Walker [:tracy]]

I just ran browser_bug581253.js in e10s and it passes.

Can we morph this bug to re-enable the test case?

Comment 2

[Jim Mathies [:jimm]]

(In reply to [:tracy] Tracy Walker - QA Mentor from comment #1)
> I just ran browser_bug581253.js in e10s and it passes.
> 
> Can we morph this bug to re-enable the test case?

Seems like the right thing to do.

Comment 3

[Tracy Walker [:tracy]]

so this would just be removing the skip-if line in browser.ini, right?

Jim, let's walk through that tomorrow and see if I can check-in the patch.  I have a feeling I didn't do all the hoop jumping a few years back to maintain my check-in rights.  Surely there is a way check if I do or not?

Comment 4

[Tracy Walker [:tracy]]

patch (https://hg.mozilla.org/try/rev/6bb4865b2a3f) passes try https://treeherder.mozilla.org/#/jobs?repo=try&revision=6bb4865b2a3f

Comment 5

[Jim Mathies [:jimm]]

How about the other tests you found? Lets morph this bug into a more general one and add those too so we can land this all at once.

Comment 6

[Tracy Walker [:tracy]]

Ok, I'll get that list together, try a new patch locally. Once I've nailed down which test cases can be re-enabled, I'll push a new patch to try and morph this bug accordingly.

Comment 7

[Tracy Walker [:tracy]]

Attachment: re-enable tests

Comment 8

[Tracy Walker [:tracy]]

Try runs look good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a5572d8b1922

This attached patch will re-enable the following tests for e10s:

browser_bug623155.js
browser_bug678392.js
browser_bug880101.js, browser_favicon_change_not_in_document.js
browser_identity_UI.js
browser_keywordSearch.js
browser_parsable_css.js
browser_searchSuggestionUI.js
browser_syncui.js
browser_tabfocus.js
browser_visibleTabs_tabPreview.js

Comment 9

[Jim Mathies [:jimm]]

How would you like to land this? On your own or through tree drivers/checkin-needed?

Comment 10

[Tracy Walker [:tracy]]

Attachment: Patch with correct header information for checkin

Comment 11

[Jim Mathies [:jimm]]

Comment on attachment 8627920 [details] [diff] [review]
Patch with correct header information for checkin

Your summary info isn't complete - 

"Bug 1093756: Re-enable several test cases under e10s"

should be something like:

"Bug 1093756: Re-enable several test cases under e10s. r=jimm"

Once that's updated, you can carry forward my r+ and add checkin-needed to the bug.

Comment 12

[Jim Mathies [:jimm]]

Also add more detail - 

"Re-enable several browser chrome tests under e10s. r=jimm"

Comment 13

[Tracy Walker [:tracy]]

Attachment: More detailed summary for checkin patch

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/0024c5cfa25b

Comment 15

[Carsten Book [:Tomcat]]

(In reply to [:tracy] Tracy Walker - QA Mentor from comment #13)
> Created attachment 8627944 [details] [diff] [review]
> More detailed summary for checkin patch

no problem Tracy, landed this as https://hg.mozilla.org/integration/fx-team/rev/0024c5cfa25b \o/

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0024c5cfa25b

Comment 17

[Carsten Book [:Tomcat]]

sorry had to back this out from mozilla-central and integration trees for perma failures on OS X like https://treeherder.mozilla.org/logviewer.html#?job_id=11418476&repo=mozilla-inbound

Comment 18

[Pulsebot]

Backout:
https://hg.mozilla.org/mozilla-central/rev/e7e69cc8c07b

Comment 19

[Tracy Walker [:tracy]]

Attachment: patch with perma-fail testcase skipped on Mac

There was only one testcase enabled here that was causing perma failures on Mac 10.10.  The new patch disables the test case for Mac.  

New try run for Mac 10.10 is green (sorry didn't run it prior to previous patch, I didn't know 10.10 has to be manual selected in the try code generator). https://treeherder.mozilla.org/#/jobs?repo=try&revision=c7c985f142e1

Comment 20

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/a4e78383fd88

Comment 21

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a4e78383fd88