1713912 - Tests skipped in 'toolkit/components/printing/tests/browser.ini' for new Fission platform triage

Status: RESOLVED FIXED

(Toolkit :: Printing, task)

Description

[Andrew Halberstadt [:ahal]]

The following tests were disabled in toolkit/components/printing/tests/browser.ini due to new Fission platform triage:
browser_modal_print.js
browser_print_in_container.js

Full diff:

diff --git a/toolkit/components/printing/tests/browser.ini b/toolkit/components/printing/tests/browser.ini
--- a/toolkit/components/printing/tests/browser.ini
+++ b/toolkit/components/printing/tests/browser.ini
@@ -3,16 +11,18 @@ support-files =
   head.js
   simplifyArticleSample.html
 
 [browser_cancel_close_print.js]
 [browser_destination_change.js]
 [browser_empty_paper_sizes.js]
 
 [browser_modal_print.js]
+skip-if =
+  fission && os == "linux" && asan  # New platform triage
 
 [browser_modal_resize.js]
 
 [browser_page_change_print_original.js]
 support-files =
   file_page_change_print_original_1.html
   file_page_change_print_original_2.html
 skip-if = os == "mac"
@@ -74,9 +84,10 @@ skip-if =
   verify && !debug && (os == "linux")
   os == "mac" 
   os == "linux" # Bug 1676659
 
 [browser_print_in_container.js]
 skip-if = 
   tsan # Bug 1683730
   os == "linux" && bits == 64 && debug # Bug 1683279 
+  os == "linux" && fission && asan  # New platform triage
 [browser_system_dialog_subdialog_hidden.js]

See the following try push for failures:
https://treeherder.mozilla.org/jobs?repo=try&revision=ad7a18fdad3b8e35b44152c16d94babb80387cd8

To run these failures in your own try push, first revert the skip-if
annotations, then run:

$ ./mach try fuzzy --rebuild 3 toolkit/components/printing/tests/browser.ini

Then use the fuzzy interface to select the task(s) which are relevant to the
removed skip-if expression(s). Please note the failure may be intermittent
and/or been fixed since the triage took place.

Comment 1

[Chris Peterson [:cpeterson]]

We need to determine whether this browser-chrome test failure should block shipping Fission. Tracking for Fission Milestone M7a for the initial audit. Then we can decide whether to move this bug to Fission M8 or post-MVP (Fission Future).

Here is a recent Try run for Fission browser-chrome:

https://treeherder.mozilla.org/jobs?repo=try&revision=ad7a18fdad3b8e35b44152c16d94babb80387cd8

Comment 2

[Chris Peterson [:cpeterson]]

Sam, here is a browser_print_in_container.js failure with Fission. This failure might be a duplicate of bug 1683279, where you skipped browser_print_in_container.js for Linux asan. Do you think this Fission failure should block shipping Fission MVP?

https://treeherder.mozilla.org/push-health/push?repo=try&revision=ad7a18fdad3b8e35b44152c16d94babb80387cd8&testGroup=pr&selectedTest=toolkitcomponentsprintingtestsbrowserprintincontainerjs&selectedTaskId=&selectedJobName=docshell%2Ftest%2Fbrowser%2Fbrowser_backforward_userinteraction.js+test-macosx1015-64-qr%2Fopt-mochitest-browser-chrome-fis-e10s-1

I don't see any browser_modal_print.js failures in recent Fission browser-chrome try runs (before these Fission skip-if annotations were added). We can probably just remove the browser_modal_print.js annotation and see what happens. We have had some false positive bug reports for Fission browser-chrome failures.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

I told neha that I'd poke at these.

Comment 4

[Daniel Holbert [:dholbert]]

I took a look too before seeing emilio's comment.

tl;dr: nothing here seems like it should block shipping Fission MVP.

(In reply to Chris Peterson [:cpeterson] from comment #2)

Sam, here is a browser_print_in_container.js failure with Fission. This failure might be a duplicate of bug 1683279, where you skipped browser_print_in_container.js for Linux asan. Do you think this Fission failure should block shipping Fission MVP?

Minor correction: it looks like we skipped browser_print_in_container.js for linux64 debug over there (not for ASAN). Whereas, the failures here are on linux opt ASAN (with fission).

In any case, it does look like we are seeing failures in that test with linux opt ASAN without fission as well -- e.g. this log from the most recent intermittent-failure-bot comment from https://bugzilla.mozilla.org/show_bug.cgi?id=1683279#c37 :
https://treeherder.mozilla.org/logviewer?job_id=341329196&repo=mozilla-beta&lineNumber=6984

So I think we can consider this test failure to be wholly unrelated to fission.

I don't see any browser_modal_print.js failures in recent Fission browser-chrome try runs (before these Fission skip-if annotations were added). We can probably just remove the browser_modal_print.js annotation and see what happens. We have had some false positive bug reports for Fission browser-chrome failures.

I haven't been able to find any failures in this test at all -- I clicked around in the try run from comment 0 and I couldn't find any mention of this test failing. (The three linux-asan bc8 test-runs from that Try run all actually show this test running & passing, in their raw logs.) So, if there were problems with this test at some point (which I'm not clear on), it's good to hear we're not seeing failures now at least. :)

So I agree we should be able to remove the fission-specific annotations here. (Seems like we can drop the annotation for browser_modal_print.js entirely, and maybe just drop the 'fission' condition for browser_print_in_container.js.

Comment 5

[Daniel Holbert [:dholbert]]

Attachment: Bug 1713912: Remove skip-if-fission annotations for tests browser_modal_print.js and browser_print_in_container.js. r?emilio

In the former case (for browser_modal_print.js), it's not clear whether the
annotation was ever needed -- the test was actually passing all the testruns in
https://bugzilla.mozilla.org/show_bug.cgi?id=1713912#c0 . So I'm just removing
the annotation entirely.

In the latter case (for browser_print_in_container.js), it looks like the test
has some known intermittent timeouts on slow configurations, and it's had some
observed timeouts on non-fission Linux-ASAN runs. So it seems likely that
our Linux+ASAN+fission annotation there was accidentally too-specific, and
fission probably wasn't a relevant subcondition there. Therefore, this patch
is just removing fission from that condition, so the test will now be skipped on
Linux ASAN testruns in general.

Comment 6

[Daniel Holbert [:dholbert]]

Try run with those annotation updates (via the attached patch):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=35b945a9b460801f1494644ed86dee018fd9e4a5

Comment 7

[Daniel Holbert [:dholbert]]

(Note, browser_modal_print.js does seem to reliably fail when I run it locally, and it doesn't seem to matter whether or not I have fission preffed on. I filed bug 1715653 on that.)

Comment 8

[Chris Peterson [:cpeterson]]

Thanks, Daniel!

I'll clear the old needinfo requests for Sam and Emilio since you have a patch.

Comment 9

[Daniel Holbert [:dholbert]]

FWIW my Try run (comment 6) did hit one test failure in browser_modal_print.js (in one out of its 3 runs), and that looks like it's simply a version of the intermittent that's tracked in bug 1700803. So, not especially concerning from a fission perspective.

Comment 10

[Daniel Holbert [:dholbert]]

I did some retriggers to follow up on that observed browser_modal_print.js failure, to see how frequent it is. I got 4 failures out of 52 runs, for Linux opt M-Fis. So that's a roughly 8% failure rate there.

In comparison, I did 50 retriggers with non-fission on the same platform (in the same try run), and got 0 failures.

So, this suggests there's indeed a higher-than-baseline failure rate there. So we probably should leave that one annotated as skipped-for-fission, for now...

Here's the Try link again, filtered to show those retriggers/failures:
https://treeherder.mozilla.org/jobs?repo=try&revision=35b945a9b460801f1494644ed86dee018fd9e4a5&searchStr=Linux%2Cx64%2Copt%2Cmochitest

Comment 11

[Daniel Holbert [:dholbert]]

So it looks like browser_modal_print.js contains or triggers some race condition, and it's a race that's easier-to-lose with Fission enabled... or something like that.

So: I'm going to leave that one annotated, and I'll update the annotation to point at bug 1700803, and I'm hoping that I can redirect emilio's comment 3 into poking at that test-failure over there to see how much we should worry about it.

Comment 12

[Sam Foster [:sfoster] (he/him)]

Sorry I had typed things in here but didn't get back to finish. But basically my findings were the same as dholbert's - the browser_print_in_container.js failures seem to just be timeouts - some parts of the test can be slow and it sometimes fails for that reason.

Comment 13

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #11)

So: I'm going to leave that one annotated, and I'll update the annotation to point at bug 1700803, and I'm hoping that I can redirect emilio's comment 3 into poking at that test-failure over there to see how much we should worry about it.

Sorry, reconsidering again: I guess the current skip-annotation there is dependent on linux+fission+ASAN. So apparently the plain "linux+fission" failure rate (the rate I was worried about in comment 10) hasn't actually been abysmally high, or else it would've caused more noise in bug 1700803.

So: I'll land the patch as-posted after all, and if the failure rate ends up being too high in some configuration, we'll find out and can take action in bug 1700803.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9d056525f029
Remove skip-if-fission annotations for tests browser_modal_print.js and browser_print_in_container.js. r=emilio

Comment 15

[Daniel Holbert [:dholbert]]

Attachment: Bug 1713912 followup: drop trailing blank space. (no review, whitespace-only)

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2f3e7cfc271b
followup: drop trailing blank space. (no review, whitespace-only) DONTBUILD

Comment 17

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/9d056525f029
https://hg.mozilla.org/mozilla-central/rev/2f3e7cfc271b