1387612 - Show Safe Browsing timeouts in Marionette logs

Status: RESOLVED FIXED

(Testing :: Firefox UI Tests, enhancement)

Description

[François Marier [:francois]]

Until bug 1386810 is available, we should hardcode MOZ_LOG=UrlClassifierStreamUpdater:1 in order to get more useful info in the Safe Browsing tests.

This is extremely low noise (nothing unless Safe Browsing times out) and Safe Browsing is disabled in almost all of our tests, so it should be pretty cheap.

Comment 1

[François Marier [:francois]]

Attachment: Bug 1387612 - Show Safe Browsing timeouts in Marionette logs.

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

Comment 2

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

Comment on attachment 8894015 [details]
Bug 1387612 - Show Safe Browsing timeouts in Marionette logs.

https://reviewboard.mozilla.org/r/165100/#review170720

::: testing/marionette/client/marionette_driver/geckoinstance.py:239
(Diff revision 1)
>          if self.headless:
>              env["MOZ_HEADLESS"] = "1"
>              env["DISPLAY"] = "77"  # Set a fake display.
>  
> +        # low-noise log messages useful in tests
> +        env["MOZ_LOG"] = "UrlClassifierStreamUpdater:1"

Any safe browsing test is running through the firefox ui harness. So lets add this env variable temporarily over there to not clutter Marionette with this.

Comment 3

[François Marier [:francois]]

(In reply to Henrik Skupin (:whimboo) from comment #2)
> So lets add this env variable temporarily over there to not clutter
> Marionette with this.

Do you mean in this function?

https://searchfox.org/mozilla-central/rev/b52285fffc13f36eca6b47de735d4e4403b3859e/testing/firefox-ui/harness/firefox_ui_harness/runners/base.py#17

I'm not familiar at all with how we're using marionette.

Comment 4

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

Yes, and it should be similar to what we currently do for update tests:
https://searchfox.org/mozilla-central/source/testing/firefox-ui/harness/firefox_ui_harness/runners/update.py#45.

Comment 5

[François Marier [:francois]]

Comment on attachment 8894015 [details]
Bug 1387612 - Show Safe Browsing timeouts in Marionette logs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/165100/diff/1-2/

Comment 6

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

Comment on attachment 8894015 [details]
Bug 1387612 - Show Safe Browsing timeouts in Marionette logs.

https://reviewboard.mozilla.org/r/165100/#review171592

::: testing/firefox-ui/harness/firefox_ui_harness/runners/base.py:26
(Diff revision 2)
>          self.app = 'fxdesktop'
>  
> +        # low-noise log messages useful in tests
> +        # TODO: should be moved to individual tests once bug 1386810
> +        # is fixed
> +        os.environ['MOZ_LOG'] = 'UrlClassifierStreamUpdater:1'

Wait. Something I totally missed is that this would override any other setting for this environment variable as set outside of the harness.

So we would have to check if it exists, and then append the URLClassifierStreamUpdater logger.

Comment 7

[François Marier [:francois]]

Comment on attachment 8894015 [details]
Bug 1387612 - Show Safe Browsing timeouts in Marionette logs.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/165100/diff/2-3/

Comment 8

[François Marier [:francois]]

Henrik, if that last revision looks good to you, please request an auto-land after you r+ it. I'm going to be on PTO for a few days.

Comment 9

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

Comment on attachment 8894015 [details]
Bug 1387612 - Show Safe Browsing timeouts in Marionette logs.

https://reviewboard.mozilla.org/r/165100/#review172642

As a workaround this looks fine to me.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a1ec0cb0728
Show Safe Browsing timeouts in Marionette logs. r=whimboo

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3a1ec0cb0728