browser_context_menu_tests.js test failures

RESOLVED FIXED in Firefox 23

Status

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: jimm, Assigned: rsilveira)

Tracking

Trunk
Firefox 23
x86_64
Windows 8.1
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_context_menu_tests.js | Top position is 167.63333129882812, expected between 175 and 190
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_context_menu_tests.js | Top position is 87.63333129882812, expected between 95 and 110
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_context_menu_tests.js | Top position is 81.68333435058594, expected between 110 and 125
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_context_menu_tests.js | Cleanup function threw an exception at chrome://browser/content/browser.js:460 - TypeError: tab.browser is null
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/browser_context_menu_tests.js | Cleanup function threw an exception at chrome://browser/content/browser.js:460 - TypeError: tab.browser is null
Summary: failing browser_context_menu_tests.js → browser_context_menu_tests.js test failures
OS: Windows 7 → Windows 8 Metro
Assignee: nobody → rsilveira
Posted patch Test fixes (obsolete) — Splinter Review
The addTab method on head.js calls registerCleanupFunction() to close the tabs after the tests are done. Some tests were closing the tabs before the cleanUp then the cleanup would throw because the tab was invalid. I've added a check to see if the tab is still valid before closing.

When closing a tab, we show the tab bar briefly. The addTab cleanup function would execute and the next test would start while the tab bar was peeking. This caused some unexpected intermittent failures too. I've added a hideContextUI() call to addTab cleanup to make sure the tab bar is not visible at the beginning of a tests.

I'm not sure why we're only seeing this now though. The peek behavior has been there for a while.

With this patch, the only failures I get are on selection tests. It also fixes Bug 864755 and Bug 864758.
Attachment #743711 - Flags: review?(jmathies)
(In reply to Jim Mathies [:jimm] from comment #0)
> TEST-UNEXPECTED-FAIL |
> chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/
> browser_context_menu_tests.js | Top position is 167.63333129882812, expected
> between 175 and 190

Does this patch fix these coordinate failures as well?
(In reply to Jim Mathies [:jimm] from comment #2)
> (In reply to Jim Mathies [:jimm] from comment #0)
> > TEST-UNEXPECTED-FAIL |
> > chrome://mochitests/content/metro/browser/metro/base/tests/mochitest/
> > browser_context_menu_tests.js | Top position is 167.63333129882812, expected
> > between 175 and 190
> 
> Does this patch fix these coordinate failures as well?

I still see these, I wonder if this has to do with my use of a hidpi system. If you aren't, I can file a follow up.
Oh could be. I'll run and fix the tests on HiDPI.
Hey Rodrigo, care to take this addition to your patch for a spin? We are really messy when it comes to cleaning up our extra tabs for tests. These can cause all sorts of problems. I think we check something like this in so we can catch offenders when patches land.
Attachment #743747 - Flags: review?(rsilveira)
Attachment #743711 - Flags: review?(jmathies) → review+
Attachment #743747 - Flags: review?(rsilveira) → review+
Posted patch Patch v2Splinter Review
We were using registerCleanupFunction to close opened tabs but it runs the cleanup only when selecting the next test to run. See https://mxr.mozilla.org/mozilla-central/source/testing/mochitest/browser-test.js#256

This means that the tabs would always leak after the main runTests() loop. We couldn't move the tabs leaking code verification to the top of runTests either because some tests were creating tabs before calling runTests(). I ended up adding code to handle tabs lifecycle withing head.js so that we have full control.

I've changed all calls to close tabs to use the foce option so that there is no contextUI displayed.

The context menu difference was happening only on HiDPI devices. The font-size property I added to textblock01.html fixed it and still works on non-HiDPI.
Attachment #743711 - Attachment is obsolete: true
Attachment #744732 - Flags: review?(jmathies)
Attachment #744732 - Flags: review?(jmathies) → review+
https://hg.mozilla.org/mozilla-central/rev/5f90a20c173e
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.