Closed Bug 1262831 Opened 4 years ago Closed 4 years ago

Enable working tests in dom/tests/browser/


(Core :: DOM: Core & HTML, defect)

Not set



Tracking Status
e10s + ---
firefox48 --- fixed


(Reporter: tracy, Assigned: tracy)


(Blocks 1 open bug)



(1 file, 1 obsolete file)

Enable browser_frame_elements.js, browser_test_content.js and browser_test_new_window_from_content.js.

Passes try (unrelated intermittent):
Summary: Enable working tests in dome/tests/browser/ → Enable working tests in dom/tests/browser/
Attached patch bug_1262831.patch (obsolete) — Splinter Review
Attachment #8739031 - Flags: review?(jmathies)
Assignee: nobody → twalker
Attachment #8739031 - Flags: review?(jmathies) → review+
Keywords: checkin-needed
(In reply to Tracy Walker [:tracy] from comment #0)
> Passes try (unrelated intermittent):

These are non-e10s test runs. Is there a Try push that shows results for the e10s suites? i.e. M-e10s
Keywords: checkin-needed
Also, you should probably retrigger the chunks that run your tests a few times to make sure there aren't any intermittents lurking in those tests.
To compliment the previous, non-e10s, try run, I just pushed an e10s only run. Let's see how it goes.
Looks like on Linux debug, browser_test_new_window_from_content.js is bumping right up against the per-test runtime limit and occasionally exceeding it (~130s with a max of 135s due requestLongerTimeout(3) already there). Non-e10s runs are closer to 70s in runtime. A ~2x regression seems pretty bad and worse than we'd typically expect. We could wallpaper over it by upping the timeout factor to 4, but it feels to me like the test could use someone looking at it a bit before that happens.
This has an intermittent bug filed;

I'll post a patch here shortly that will leave browser_test_new_window_from_content.js disabled for Linux debug.
This patch disables browser_test_new_window_from_content.js for Linux debug.

Try run passes all other cases.
Attachment #8739031 - Attachment is obsolete: true
Attachment #8740127 - Flags: review?(ryanvm)
Comment on attachment 8740127 [details] [diff] [review]

Review of attachment 8740127 [details] [diff] [review]:

::: dom/tests/browser/browser.ini
@@ +37,4 @@
>  [browser_localStorage_privatestorageevent.js]
>  skip-if = e10s
>  [browser_test_new_window_from_content.js]
> +skip-if = (toolkit == 'android' || buildapp == 'b2g' || buildapp == 'mulet') || (os == "linux" && debug) # see bug 1261495 for Linux debug time outs

Sure, I guess this works.
Attachment #8740127 - Flags: review?(ryanvm) → review+
Keywords: checkin-needed
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.