Closed Bug 1358665 Opened 5 years ago Closed 5 years ago

close_all_tabs() doesn't find tab elements in _check_and_fix_leaked_handles()

Categories

(Toolkit :: Telemetry, defect)

defect
Not set
normal

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.
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.
Summary: JavascriptException: Element reference not seen before after restarting browser → close_all_tabs() doesn't find tab elements in _check_and_fix_leaked_handles()
(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.
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: nobody → jdorlus
Flags: needinfo?(ato)
I have come to find that the element that seems to be providing the problem is the tabbar. That is the element referenced.
OK, clearing my needinfo in that case.
Flags: needinfo?(ato)
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 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-
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 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+
Component: Marionette → Telemetry
Product: Testing → Toolkit
Version: Version 3 → unspecified
Pushed by chutten@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5c679e572655
Fixed test to use puppeteer to restart;r=chutten
Flags: needinfo?(jdorlus)
https://hg.mozilla.org/mozilla-central/rev/5c679e572655
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
No longer blocks: 1355818
You need to log in before you can comment on or make changes to this bug.