1329683 - Test additions for test_navigation.py on bug 1328676 breaks tests for Fennec

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Earlier today I noticed that we have again a bump in failing tests for Fennec on autoland (Chunk 4) since a couple of days ago.

https://treeherder.mozilla.org/logviewer.html#?job_id=67247736&repo=autoland&lineNumber=1921

Checking for changes of this file I noticed that with bug 1328676 a new test got added which should not run if Marionette doesn't handle the instance, and also not on Fennec:

https://hg.mozilla.org/mozilla-central/rev/8c25f4454f5b#l2.12

With the new decorator as added in bug 1326047 this can easily be done.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1329683 - Run test_focus_after_navigation only if Marionette manages the instance.

The test test_focus_after_navigation should only be run if Marionette manages the instance
of the application itself. Otherwise all remaining tests in chunk 4 are broken too.

Review commit: https://reviewboard.mozilla.org/r/103322/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/103322/

Comment 2

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8825082 [details]
Bug 1329683 - Run test_focus_after_navigation only if Marionette manages the instance.

https://reviewboard.mozilla.org/r/103322/#review103942

::: testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py:187
(Diff revision 1)
> +    @run_if_manage_instance("Only runnable if Marionette manages the instance")
> +    @skip_if_mobile("Bug 1322993 - Missing temporary folder")
>      def test_focus_after_navigation(self):
>          self.marionette.quit()
>          self.marionette.start_session()
>  
>          self.marionette.navigate(inline("<input autofocus>"))
>          active_el = self.marionette.execute_script("return document.activeElement")
>          focus_el = self.marionette.find_element(By.CSS_SELECTOR, ":focus")
>          self.assertEqual(active_el, focus_el)

Why is this test limited to instances of Firefox tat is managed by the harness?

I get why it’s ignored on mobile (because it uses tabbrowser and XUL functionality that isn’t available on Fennec).

Comment 3

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8825082 [details]
Bug 1329683 - Run test_focus_after_navigation only if Marionette manages the instance.

https://reviewboard.mozilla.org/r/103322/#review103944

Comment 4

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8825082 [details]
Bug 1329683 - Run test_focus_after_navigation only if Marionette manages the instance.

https://reviewboard.mozilla.org/r/103322/#review103942

> Why is this test limited to instances of Firefox tat is managed by the harness?
> 
> I get why it’s ignored on mobile (because it uses tabbrowser and XUL functionality that isn’t available on Fennec).

`self.marionette.quit()` requires an instance.

Comment 5

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8825082 [details]
Bug 1329683 - Run test_focus_after_navigation only if Marionette manages the instance.

https://reviewboard.mozilla.org/r/103322/#review103956

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 8825082 [details]
Bug 1329683 - Run test_focus_after_navigation only if Marionette manages the instance.

https://reviewboard.mozilla.org/r/103322/#review103942

> `self.marionette.quit()` requires an instance.

As Maja already said, if Marionette doesn't manage the instance it has no path to the binary, and as such cannot launch the application again. Same applies for forced restarts. An in_app restart would be possible but that's not available yet for mobile. 

So for now it would really be the best to skip it for both conditions. Also with having bug 1322993 in mind.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/27e89a88acff
Run test_focus_after_navigation only if Marionette manages the instance. r=maja_zf

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/27e89a88acff

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

M4 chunk for Fennec look fine again. Please uplift this test-only patch to aurora.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9067cd1a7282