Closed Bug 1379292 Opened 6 years ago Closed 6 years ago

Telemetry Harness should look at ping type and reason

Categories

(Toolkit :: Telemetry, enhancement)

Product:
Toolkit
Component:
Telemetry
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla56
Tracking Flags:
Tracking Status
firefox56 --- fixed

People

(Reporter: Silne30, Assigned: Silne30)

Details

Attachments

(2 files)

Bug 1379292 - Added filtering for pings based on type and reason
59 bytes, text/x-review-board-request
chutten
: review+
Details
Bug 1379292 - Made changes to test to use new harness filtering
59 bytes, text/x-review-board-request
chutten
: review+
Details 
Right now the telemetry harness only handles a single ping when it comes in. The problem is that multiple ping types have been added that are sent by the same event. I.e. new-profile ping and main ping can be sent on shutdown. Furthermore, pings can be triggered and sent for different reasons. We need to have the harness wait for the proper ping sent for the right reason and filter out the others.
Assignee: nobody → jdorlus
Status: NEW → ASSIGNED
Attachment #8884977 - Flags: review?(chutten)
Attachment #8884978 - Flags: review?(chutten)
Comment on attachment 8884977 [details]
Bug 1379292 - Added filtering for pings based on type and reason

https://reviewboard.mozilla.org/r/155800/#review160932

::: toolkit/components/telemetry/tests/marionette/harness/telemetry_harness/testcase.py:53
(Diff revision 1)
>          }
>  
>          # Firefox will be forced to restart with the prefs enforced.
>          self.marionette.enforce_gecko_prefs(telemetry_prefs)
>  
> -    def wait_for_ping(self):
> +    def wait_for_ping(self, ping_info):

Could you take a lambda instead? Then callers could specify exactly the filter criteria they need.
Comment on attachment 8884978 [details]
Bug 1379292 - Made changes to test to use new harness filtering

https://reviewboard.mozilla.org/r/155802/#review160936

::: commit-message-a5f3f:3
(Diff revision 1)
> +Bug 1379292 - Made changes to test to use new harness filtering
> +
> +Self explanitory. Also added some waits for test hardiness.

*explanatory

::: toolkit/components/telemetry/tests/marionette/tests/client/test_main_tab_scalars.py:35
(Diff revision 1)
>          assert ping['clientId'] == self.client_id
>          scalars = ping['payload']['processes']['parent']['scalars']
>          assert scalars['browser.engagement.max_concurrent_tab_count'] == 3
> -        assert scalars['browser.engagement.tab_open_event_count'] == 2
>          assert scalars['browser.engagement.max_concurrent_window_count'] == 1
> +        assert scalars['browser.engagement.tab_open_event_count'] == 2

Why did you rearrange this?
Comment on attachment 8884978 [details]
Bug 1379292 - Made changes to test to use new harness filtering

https://reviewboard.mozilla.org/r/155802/#review160936

> *explanatory

Noted.

> Why did you rearrange this?

Oh, yes. This was part of debugging. Reverting.
Comment on attachment 8884977 [details]
Bug 1379292 - Added filtering for pings based on type and reason

https://reviewboard.mozilla.org/r/155800/#review161220
Attachment #8884977 - Flags: review?(chutten) → review+
Comment on attachment 8884978 [details]
Bug 1379292 - Made changes to test to use new harness filtering

https://reviewboard.mozilla.org/r/155802/#review161224
Attachment #8884978 - Flags: review?(chutten) → review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e123b6e47340
Added filtering for pings based on type and reason r=chutten
https://hg.mozilla.org/integration/autoland/rev/064f2ad7ca3b
Made changes to test to use new harness filtering r=chutten
Keywords: checkin-needed
Backed out for linting failures in test_main_tab_scalars.py:

https://hg.mozilla.org/integration/autoland/rev/0580a54610160c86c488010e00c4b89a61537250
https://hg.mozilla.org/integration/autoland/rev/85332a69ae0cbad6556541883960c517b1b86a03

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=064f2ad7ca3bcd37df0ca60dd870a722ca96c756&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=113397099&repo=autoland

TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/toolkit/components/telemetry/tests/marionette/tests/client/test_main_tab_scalars.py:30:45 | continuation line over-indented for visual indent (E127)
TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/toolkit/components/telemetry/tests/marionette/tests/client/test_main_tab_scalars.py:38:1 | blank line at end of file (W391)
Flags: needinfo?(jdorlus)
Made fixes for pep8 errors and resubmited.
Flags: needinfo?(jdorlus)
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/0312f3176ca8
Added filtering for pings based on type and reason r=chutten
https://hg.mozilla.org/integration/autoland/rev/61e56e860c50
Made changes to test to use new harness filtering r=chutten
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/0312f3176ca8
https://hg.mozilla.org/mozilla-central/rev/61e56e860c50
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.