Closed Bug 1114710 Opened 8 years ago Closed 8 years ago

test_leaf_layers_partition_browser_window.xul fails when run as a standalone directory


(Core :: Layout, defect)

Not set





(Reporter: vaibhav1994, Assigned: jmaher)




(1 file, 1 obsolete file)

in bug 1110982, we are looking to enable tests where we run a fresh browser instance per directory.  Usually what happens is that a few tests fail because they accidentally depend on the state of the browser from an earlier test.

In the try run:

In linux debug 'oth' chunk, the test fails as following:

2166 INFO TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/layout/base/tests/chrome/test_leaf_layers_partition_browser_window.xul | Test timed out. - expected PASS
in this case we are running:
./mach mochitest-chrome layout/base/tests/chrome

note: this only runs on linux* debug builds, as per the manifest.
When I try to get the logs from the try server push I get a not found error.

Does the test pass if you insert a do-nothing test before it that just waits for say 10 seconds or so?
Did another try push:

And the test is no longer failing. Closing this bug as of now.
Closed: 8 years ago
Resolution: --- → FIXED
Unfortunately, this test is failing as an intermittent again specifically on Linux Debug.

This is the try push:

The error:
 2157 INFO TEST-UNEXPECTED-FAIL | layout/base/tests/chrome/test_leaf_layers_partition_browser_window.xul | Test timed out. - expected PASS

From the logs:

16:40:27     INFO -  2155 INFO TEST-PASS | layout/base/tests/chrome/test_leaf_layers_partition_browser_window.xul | Leaf layers should form a non-overlapping partition of the browser window (non-maximized)
16:40:27     INFO -  2156 INFO Maximizing test 1 window (maximized)
16:40:27     INFO -  2157 INFO TEST-UNEXPECTED-FAIL | layout/base/tests/chrome/test_leaf_layers_partition_browser_window.xul | Test timed out. - expected PASS
16:40:27     INFO -  2158 INFO TEST-OK | layout/base/tests/chrome/test_leaf_layers_partition_browser_window.xul | took 308748ms
Resolution: FIXED → ---
Blocks: 1110982
Attached patch change the test (obsolete) — Splinter Review
Looking at the test I don't see any reason there needs to be a paint after we get the load event. The page could have been painted before the load event.

This patch makes us skip requiring a paint to happen after the load event (but keeps that requirement for after we get the resize event from the maximize call).

Can you give this a try to see if it fixes the problem?
Flags: needinfo?(vaibhavmagarwal)
Thanks for the quick patch-

pushed to try:
Flags: needinfo?(vaibhavmagarwal)
:tn, thanks for looking into this and repushing.  I see that the second patch+try push didn't yield success.  Do let me know if I can do anything to help out.
I pushed a few more try pushes that I didn't paste here. Here's the next one

So far it looks like the test is started by the test runner, and the nothing at all happens for five minutes. The test does not get a load event, and an inline script in the test does not even execute. Then the test harness takes a screenshot with screentopng, this seems to actually get the test to start and the load event etc are fired, the test seems to proceed normally, but it's too late as the test harness has already decided the test has timed out and it kills it.
interesting.  Is there a focus issue somewhere?  I see that we do a call, then wait on the load event.  Could that be part of the problem?  

What is odd is this only fails when we run the layout/base/tests/chrome/ directory by itself instead of everything in a single collection.  Knowing that this fails to get a load event, maybe a previous test in another directory is maximizing the browser or changing a pref/state which allows this to load properly.

I will create a linux32 vm and look at this locally.  Maybe there is something simple to fix.
I am able to reproduce this on a linux vm (ubuntu 14.04 fresh install), after downloading the build/test from:

then I ran the tests and saw the error.  I will poke around some more to see if I can make more sense of it all!
oh, this is frustrating, I can easily reproduce this, but I see a variety of issues.

* if my terminal where I run the python command line is maximized, the test passes, I suspect this is related to the scrolling output.
* if my terminal where I run the python command is regular sized (ends up being hidden behind the firefox window locally), the test hangs
* test_chrome_over_plugin.xul also exhibits odd behaviour in terminal maximized/minimized state (opposite of test_leaf_layers_partition_browser_window.xul)
oh this is frustrating-

this fails (what is run via run-by-dir):
python ... --test-path layout/base/tests/chrome

but this passes (same exact set of tests):
python ... --test-path layout/base/tests

very odd.  What is interesting is that the failure case yields maximizing the main browser with the simpletest harness instead of the popup window.  That indicates a focus issue.  Still not sure if this is the root cause, but I will look into this a bit further.
I did some hacking, and all avenues lined up to focus, so I forced the focus, verified it locally in many cases and pushed to try, green (the one orange is a different test/ different directory):
Assignee: nobody → jmaher
Attachment #8543037 - Flags: review?(tnikkel)
Attachment #8542311 - Attachment is obsolete: true
Does using waitForFocus not fix it? We have to force it?
ah, of course I should have tried that.
I tried a SimpleTest.waitForFocus(windowSetup, win) instead of setTimeout(windowSetup, 0) and it didn't work.  I would get the same behavior where the main window would get maximized and stay there.

Alternatively we could use:
SimpleTest.executeSoon(windowSetup) instead of setTimeout(windowSetup, 0)

I am open to more hacking
The call here to maximize just uses the most recent window (whatever that means), so it's likely just using whatever has focus. So just ensuring that the child window has focus at that point should be enough to fix the wrong window getting maximized.
Attachment #8543037 - Flags: review?(tnikkel) → review+
Keywords: checkin-needed
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.