Closed
Bug 1358665
Opened 7 years ago
Closed 7 years ago
close_all_tabs() doesn't find tab elements in _check_and_fix_leaked_handles()
Categories
(Toolkit :: Telemetry, defect)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: Silne30, Assigned: Silne30)
Details
(Keywords: pi-marionette-firefox-puppeteer)
Attachments
(1 file)
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36 Steps to reproduce: def test_main_tab_scalars(self): with self.marionette.using_context(self.marionette.CONTEXT_CHROME): tab2 = self.browser.tabbar.open_tab() self.browser.tabbar.switch_to(tab2) self.browser.tabbar.close_tab(tab2, force=True) self.marionette.restart(in_app=True, clean=False) assert 1 == 1 Actual results: TEST-UNEXPECTED-ERROR | test_simple_test.py TestMainTabScalars.test_main_tab_scalars | JavascriptException: Element reference not seen before: 5e42b3dd-44bd-4a75-8589-ac0c13dfbf95 Traceback (most recent call last): File "/home/jdorlus/Desktop/firefox/testing/marionette/harness/marionette_harness/marionette_test/testcases.py", line 197, in run self.tearDown() File "/home/jdorlus/Desktop/firefox/toolkit/components/telemetry/tests/marionette/harness/telemetry_harness/testcase.py", line 100, in tearDown super(TelemetryTestCase, self).tearDown() File "/home/jdorlus/Desktop/firefox/testing/marionette/puppeteer/firefox/firefox_puppeteer/mixins.py", line 99, in tearDown self._check_and_fix_leaked_handles() File "/home/jdorlus/Desktop/firefox/testing/marionette/puppeteer/firefox/firefox_puppeteer/mixins.py", line 60, in _check_and_fix_leaked_handles self.browser.tabbar.close_all_tabs([self.browser.tabbar.tabs[0]]) File "/home/jdorlus/Desktop/firefox/testing/marionette/puppeteer/firefox/firefox_puppeteer/ui/browser/tabbar.py", line 45, in tabs tabs = self.toolbar.find_elements(By.TAG_NAME, 'tab') File "/home/jdorlus/Desktop/firefox/testing/marionette/client/marionette_driver/marionette.py", line 59, in find_elements return self.marionette.find_elements(method, target, self.id) File "/home/jdorlus/Desktop/firefox/testing/marionette/client/marionette_driver/marionette.py", line 1964, in find_elements "findElements", body, key="value" if self.protocol == 1 else None) File "/home/jdorlus/Desktop/firefox/testing/marionette/client/marionette_driver/decorators.py", line 23, in _ return func(*args, **kwargs) File "/home/jdorlus/Desktop/firefox/testing/marionette/client/marionette_driver/marionette.py", line 735, in _send_message self._handle_error(err) File "/home/jdorlus/Desktop/firefox/testing/marionette/client/marionette_driver/marionette.py", line 768, in _handle_error raise errors.lookup(error)(message, stacktrace=stacktrace) Expected results: Test should have passed.
Comment 1•7 years ago
|
||
The problem here is that the second tab which was opened gets closed via the above code. But it does not switch to the other tab which is still open, before the tearDown code. So _check_and_fix_leaked_handles() is run and seems to fail to find the tabs.
Keywords: ateam-marionette-firefox-puppeteer
Summary: JavascriptException: Element reference not seen before after restarting browser → close_all_tabs() doesn't find tab elements in _check_and_fix_leaked_handles()
Assignee | ||
Comment 2•7 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #1) > The problem here is that the second tab which was opened gets closed via the > above code. But it does not switch to the other tab which is still open, > before the tearDown code. So _check_and_fix_leaked_handles() is run and > seems to fail to find the tabs. I just amended the test to switch to the original tab that is still opened before the restart and the issue persists even with the switching prior. Let me know if there is something else I should have done.
Comment 3•7 years ago
|
||
If you can reproduce it would be great if you can provide a fix for this issue. I don't have the time to do it myself for the moment.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → jdorlus
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(ato)
Assignee | ||
Comment 4•7 years ago
|
||
I have come to find that the element that seems to be providing the problem is the tabbar. That is the element referenced.
Comment 6•7 years ago
|
||
John, do you have an update for us? Looks like this bug is causing a problem for our update tests on beta, and we have to get this fixed asap.
Blocks: 1355818
Flags: needinfo?(jdorlus)
Comment hidden (mozreview-request) |
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8868201 [details] Bug 1358665 - Fixed test to use puppeteer to restart; https://reviewboard.mozilla.org/r/139786/#review143116 ::: toolkit/components/telemetry/tests/marionette/tests/client/test_main_tab_scalars.py:20 (Diff revision 1) > tab3 = self.browser.tabbar.open_tab() > self.browser.tabbar.switch_to(tab3) > self.browser.tabbar.close_tab(tab3, force=True) > self.browser.tabbar.close_tab(tab2, force=True) > - self.install_addon() > + self.browser.tabbar.switch_to(tab1) > + self.restart_browser() To me this seems to be a wild change. Why would installing the addon be replaced with restarting the browser?
Attachment #8868201 -
Flags: review?(chutten) → review-
Assignee | ||
Comment 9•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8868201 [details] Bug 1358665 - Fixed test to use puppeteer to restart; https://reviewboard.mozilla.org/r/139786/#review143116 > To me this seems to be a wild change. Why would installing the addon be replaced with restarting the browser? This was supposed to be the original behavior but the restart functionality had a bug. This was implemented so there was still test coverage for the ping data.
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8868201 [details] Bug 1358665 - Fixed test to use puppeteer to restart; https://reviewboard.mozilla.org/r/139786/#review143164 r+ with the comment in the commit message
Attachment #8868201 -
Flags: review- → review+
Updated•7 years ago
|
Component: Marionette → Telemetry
Product: Testing → Toolkit
Version: Version 3 → unspecified
Comment 11•7 years ago
|
||
Pushed by chutten@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5c679e572655 Fixed test to use puppeteer to restart;r=chutten
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(jdorlus)
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5c679e572655
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•