Closed Bug 1221159 Opened 5 years ago Closed 5 years ago

re-enable a few working tests for e10s

Categories

(Core :: General, defect)

44 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
e10s + ---
firefox46 --- fixed

People

(Reporter: tracy, Assigned: tracy)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

I've found a few test cases in /browser/base/content/test/general/ that are working in e10s:

browser_canonizeURL.js (Bug 1094510)
browser_contextSearchTabPosition.js (bug 967013 and bug 1094761)
browser_datachoices_notification.js (bug 1113930)
browser_popup_blocker.js (bug 1081925 and bug 112552)

try run shows they should be good to be re-enable on all platforms: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7a76da23e34e
Attached patch browser-ini-patch (obsolete) — Splinter Review
Patch attached with r=jimm
Attachment #8682552 - Flags: review?(jmathies)
Assignee: nobody → twalker
Comment on attachment 8682552 [details] [diff] [review]
browser-ini-patch

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

::: browser/base/content/test/general/browser.ini
@@ -357,5 @@
>  [browser_plainTextLinks.js]
>  [browser_popupUI.js]
>  skip-if = buildapp == 'mulet'
>  [browser_popup_blocker.js]
> -skip-if = (os == 'linux') || (e10s && debug) # Frequent bug 1081925 and bug 1125520 failures

this was disabled under linux generally (the os == 'linux' check), did you intend to turn this back on for that platform?
Attachment #8682552 - Flags: review?(jmathies) → review+
(In reply to Jim Mathies [:jimm] from comment #2)
> Comment on attachment 8682552 [details] [diff] [review]
> browser-ini-patch
> 
> Review of attachment 8682552 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/base/content/test/general/browser.ini
> @@ -357,5 @@
> >  [browser_plainTextLinks.js]
> >  [browser_popupUI.js]
> >  skip-if = buildapp == 'mulet'
> >  [browser_popup_blocker.js]
> > -skip-if = (os == 'linux') || (e10s && debug) # Frequent bug 1081925 and bug 1125520 failures
> 
> this was disabled under linux generally (the os == 'linux' check), did you
> intend to turn this back on for that platform?

Yes, I ran it through try with it enabled on Linux. It appeared to be fine in the try run.
Keywords: checkin-needed
In the try run I thought the bc7 bustage was not associated with my patch. It seems bug 1217332 may be blocking this.

I'll try to sort out which case enabled in my patch, if any, is causing that later test case, browser_syncui.js, to fail on Mac.
Depends on: 1217332
This has nothing whatsoever to do with that. You were backed out for the Linux e10s bc7 bustage in browser_canonizeURL.js, where it hits the network only with e10s, just like the comment disabling it said, like https://treeherder.mozilla.org/logviewer.html#?job_id=16836406&repo=mozilla-inbound. Your try run didn't even run Linux e10s bc7, and thus didn't run browser_canonizeURL.js.
No longer depends on: 1217332
My try run did run Linux e10s bc7. It did not, however, run any test for Linux x64.  I'll make sure browser_canonizeURL.js remains disabled for linux in a new patch.
circling back around here with an updated patch.  try run with the new set of test cases to re-enable is clean: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6e9ba8b38969
Attachment #8705275 - Flags: review?(jmathies)
Attachment #8705275 - Flags: review?(jmathies) → review+
Keywords: checkin-needed
Attachment #8682552 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/c4de716628d2
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.