Show Safe Browsing timeouts in Marionette logs

RESOLVED FIXED in Firefox 57

Status

Testing
Firefox UI Tests
RESOLVED FIXED
4 months ago
3 months ago

People

(Reporter: francois, Assigned: francois)

Tracking

(Depends on: 1 bug)

Version 3
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

4 months ago
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 hidden (mozreview-request)

Comment 2

3 months ago
mozreview-review
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.
Attachment #8894015 - Flags: review?(hskupin) → review-
(Assignee)

Comment 3

3 months ago
(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.
Flags: needinfo?(hskupin)
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.
Flags: needinfo?(hskupin)
Comment hidden (mozreview-request)

Comment 6

3 months ago
mozreview-review
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.
Attachment #8894015 - Flags: review?(hskupin) → review-
Comment hidden (mozreview-request)
(Assignee)

Comment 8

3 months ago
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

3 months ago
mozreview-review
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.
Attachment #8894015 - Flags: review?(hskupin) → review+

Comment 10

3 months ago
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3a1ec0cb0728
Show Safe Browsing timeouts in Marionette logs. r=whimboo

Comment 11

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3a1ec0cb0728
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.