Autophone - add complete dom/media tests

RESOLVED FIXED

Status

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jya, Assigned: bc)

Tracking

(Blocks: 1 bug)

unspecified
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed)

Details

(Whiteboard: [leave open][stockwell fixed])

Attachments

(11 attachments, 2 obsolete attachments)

157.36 KB, patch
jmaher
: review+
Details | Diff | Splinter Review
12.68 KB, patch
jmaher
: review+
dminor
: review+
Details | Diff | Splinter Review
13.98 KB, patch
dminor
: review+
Details | Diff | Splinter Review
14.02 KB, patch
Details | Diff | Splinter Review
14.02 KB, patch
dminor
: review+
Details | Diff | Splinter Review
80.25 KB, text/plain
Details
124.83 KB, text/plain
Details
12.57 KB, patch
Details | Diff | Splinter Review
12.57 KB, patch
Details | Diff | Splinter Review
51 bytes, text/x-github-pull-request
Details | Review | Splinter Review
51 bytes, text/x-github-pull-request
Details | Review | Splinter Review
(Reporter)

Description

2 years ago
As far as I can tell, none of the dom/media/mediasource/test mochitests are being run.

Here is a log where autophone tests are run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=da2bee1a39dfdc1fdfda2981b68982bc2ef980b0&filter-tier=1&filter-tier=2&filter-tier=3

logs for Mdm:
https://autophone.s3.amazonaws.com/v1/task/P5_AQvldThe4IPmDAVS8LQ/runs/0/artifacts/public/build/mochitest-dom-media-mochitests-dom-media-settings.ini-1-nexus-6p-11-70222d20-4664-4eec-931b-7d38508ae58d.log

logs for Mm:
https://autophone.s3.amazonaws.com/v1/task/P5_AQvldThe4IPmDAVS8LQ/runs/0/artifacts/public/build/mochitest-media-mochitests-media.ini-1-nexus-6p-11-52a1a5c2-2ef0-40b0-94d7-d05a690c2567.log

in there we can see the various dom/media/test being run.

But nothing about dom/media/mediasource

So apparently, we have no coverage of the mediasource code on android (they are disabled on android 4.3 simulator due to the slowness of the thing)
(Assignee)

Comment 1

2 years ago
The media tests which are currently defined for Autophone are:

Mdm: dom/media/test/mochitest.ini
Mm:  testing/mochitest/manifests/autophone-media.ini
Mw:  testing/mochitest/manifests/autophone-webrtc.ini
Cw:  testing/crashtest/autophone-crashtest-webrtc.list

We could add an additional test for dom/media/mediasource/test/mochitest.ini
Component: Treeherder → Autophone
Product: Tree Management → Testing
Version: --- → unspecified
(Assignee)

Comment 2

2 years ago
I looked into including the option of running all of the tests defined under dom/media. This test run includes an example for all of the mochitest manifests.

https://treeherder.allizom.org/#/jobs?repo=mozilla-central&revision=891981e67948aaebf7a63bba5181ef0a538ce163&filter-searchStr=autophone

Mdm  dom/media/test/mochitest.ini
Mdm1 dom/media/tests/mochitest/mochitest.ini
Mdm2 dom/media/mediasource/test/mochitest.ini
Mdm3 dom/media/tests/mochitest/identity/mochitest.ini
Mdm4 dom/media/webaudio/test/mochitest.ini
Mdm5 dom/media/webaudio/test/blink/mochitest.ini
Mdm6 dom/media/webspeech/recognition/test/mochitest.ini
Mdm7 dom/media/webspeech/synth/test/mochitest.ini
Mdm8 dom/media/webspeech/synth/test/startup/mochitest.ini

It is still lacking the crash tests but that can be added pretty easily. I'm thinking of:

Cdm  dom/media/test/crashtests/crashtests.list
Cdm1 dom/media/tests/crashtests/crashtests.list
Cdm2 dom/media/mediasource/test/crashtests/crashtests.list
Cdm3 dom/media/webspeech/synth/crashtests/crashtests.list

dminor: We could eliminate the autophone only Cw and Mw webrtc tests if we ran the default manifests. What do you think? We'd have to make sure to annotate the default manifests with the appropriate skips, but I think that would be the more appropriate route. Is there an advantage of keeping Cw, Mw?

jya: This would fit your current request for mediasource. Any other thoughts about adding the additional tests?

snorp: Thoughts?

jmaher, gbrown: Thoughts on naming, etc?
Flags: needinfo?(snorp)
Flags: needinfo?(jyavenard)
Flags: needinfo?(jmaher)
Flags: needinfo?(gbrown)
Flags: needinfo?(dminor)
We run more tests on Autophone than we do on the emulators (at least the last time I checked), so we'd need to be able to distinguish between an Autophone run and an emulator run in the manifests. Assuming that's not a problem, I don't see any reason to keep separate manifests.
Flags: needinfo?(dminor)
I don't like |Cdm Cdm1 Cdm2 Cdm3|, I would prefer if we have numbered jobs that all jobs with the same letters have a corresponding number, so |Cdm1 Cdm2 Cdm3 Cdm4|
Flags: needinfo?(jmaher)
(Assignee)

Comment 5

2 years ago
I did that to match the already existing Mdm job. Would you be ok with changing all of the Mdm and Cdm to numbered jobs as well? Or should we just leave Mdm alone?
change might be confusing, but leaves less questions.  Happy to take the route of least resistance :)
(Reporter)

Comment 7

2 years ago
So long that the content of what is being run doesn't change across run and between debug/release like it does for the web platform tests.

That made the wpt choice totally useless as you can't ever predict where one particular test will run, could be 1, 2, 3 or 5 etc

Otherwise, sounds good, though why mdm vs mdm1/mdm2 etc? why not number them all?
Flags: needinfo?(jyavenard)
(Assignee)

Comment 8

2 years ago
I had existing bugs with Mdm etc. But I think I'll go ahead and bite the bullet and do them all.
Flags: needinfo?(gbrown)
(Assignee)

Comment 9

2 years ago
Morph this for full coverage of dom/media
Summary: dom/media/mediasource/test mochitest aren't run → Autophone - add complete dom/media tests
Great!
Flags: needinfo?(snorp)
(Assignee)

Comment 11

2 years ago
Example runs of the full set of tests using patches for the test manifests in inbound/55, aurora/54, beta/54 and release/53. The autophone test manifests should only run the green only tests modulo the intermittent failures.

https://treeherder.allizom.org/#/jobs?repo=try&author=bclary@mozilla.com&fromchange=74120ddfd810582322c688881d9aded924baa28e&exclusion_profile=false&tochange=5877457eef7730567fb0a49fe62aad1cd92a0440

I'll attach the in tree manifest changes next.
Attachment #8860014 - Flags: review?(jmaher)
(Assignee)

Updated

2 years ago
Assignee: nobody → bob
Status: NEW → ASSIGNED
(Assignee)

Comment 12

2 years ago
This patch syncs the skips etc from the autophone webrtc tests into the normal manifests.

Try run look ok in that I don't seen any issues with the manifests.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dbc3415780480c0c2065cf86f11d6a10ad55ed89
Attachment #8860017 - Flags: review?(jmaher)
Attachment #8860017 - Flags: review?(dminor)
(Assignee)

Comment 13

2 years ago
The autophone webrtc and media tests are no longer needed now that we have full coverage.
Attachment #8860021 - Flags: review?(dminor)
(Assignee)

Comment 14

2 years ago
beta version of the patch.
Attachment #8860025 - Flags: review?(jmaher)
Attachment #8860025 - Flags: review?(dminor)
(Assignee)

Comment 15

2 years ago
beta version of the patch.
(Assignee)

Comment 16

2 years ago
release version of the patch.
Attachment #8860027 - Flags: review?(jmaher)
Attachment #8860027 - Flags: review?(dminor)
(Assignee)

Comment 17

2 years ago
release version of the patch.
Attachment #8860028 - Flags: review?(dminor)
(Assignee)

Comment 18

2 years ago
current test assignments.
(Assignee)

Comment 19

2 years ago
assignments after.
(Assignee)

Comment 20

2 years ago
PS. I do have aurora versions of the patches (they are now identical to the beta versions), but I don't think they are necessary as aurora is doa.
Comment on attachment 8860014 [details] [diff] [review]
bug-1352333-autophone-v1.patch

Review of attachment 8860014 [details] [diff] [review]:
-----------------------------------------------------------------

lots of rubber stamping here.   As I see Aurora coming out of the configs, do we get any real value from mozilla-release?  I know there is not much volume there, but it could save a little bit as well.
Attachment #8860014 - Flags: review?(jmaher) → review+
Comment on attachment 8860017 [details] [diff] [review]
bug-1352333-sync-autophone-webrtc-manifests-inbound.patch

Review of attachment 8860017 [details] [diff] [review]:
-----------------------------------------------------------------

there is a TODO for a bug filed, please sort that out so we don't need to land more pushes to update a comment; likewise ensure a try push for the emulator runs.
Attachment #8860017 - Flags: review?(jmaher) → review+
Comment on attachment 8860025 [details] [diff] [review]
bug-1352333-sync-autophone-webrtc-manifests-beta.patch

Review of attachment 8860025 [details] [diff] [review]:
-----------------------------------------------------------------

sort out the TODO item :)
Attachment #8860025 - Flags: review?(jmaher) → review+
Comment on attachment 8860027 [details] [diff] [review]
bug-1352333-sync-autophone-webrtc-manifests-release.patch

Review of attachment 8860027 [details] [diff] [review]:
-----------------------------------------------------------------

and this has the same TODO message as the other branches manifest changes :)
Attachment #8860027 - Flags: review?(jmaher) → review+
Comment on attachment 8860017 [details] [diff] [review]
bug-1352333-sync-autophone-webrtc-manifests-inbound.patch

Review of attachment 8860017 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm
Attachment #8860017 - Flags: review?(dminor) → review+
Comment on attachment 8860021 [details] [diff] [review]
bug-1352333-remove-autophone-only-webrtc-media-inbound.patch

Review of attachment 8860021 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm
Attachment #8860021 - Flags: review?(dminor) → review+
Comment on attachment 8860025 [details] [diff] [review]
bug-1352333-sync-autophone-webrtc-manifests-beta.patch

Review of attachment 8860025 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm
Attachment #8860025 - Flags: review?(dminor) → review+
Comment on attachment 8860027 [details] [diff] [review]
bug-1352333-sync-autophone-webrtc-manifests-release.patch

Review of attachment 8860027 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm
Attachment #8860027 - Flags: review?(dminor) → review+
Comment on attachment 8860028 [details] [diff] [review]
bug-1352333-remove-autophone-only-webrtc-media-release.patch

Review of attachment 8860028 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm
Attachment #8860028 - Flags: review?(dminor) → review+
(Assignee)

Comment 30

2 years ago
(In reply to Joel Maher ( :jmaher) from comment #21)
> Comment on attachment 8860014 [details] [diff] [review]
> bug-1352333-autophone-v1.patch
> 
> Review of attachment 8860014 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> lots of rubber stamping here.   As I see Aurora coming out of the configs,
> do we get any real value from mozilla-release?  I know there is not much
> volume there, but it could save a little bit as well.

I consider it a safe guard to not let anything get by us before it is released. I would hate to miss a regression.(In reply to Joel Maher ( :jmaher) from comment #22)
> Comment on attachment 8860017 [details] [diff] [review]
> bug-1352333-sync-autophone-webrtc-manifests-inbound.patch
> 
> Review of attachment 8860017 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> there is a TODO for a bug filed, please sort that out so we don't need to
> land more pushes to update a comment; likewise ensure a try push for the
> emulator runs.

Thanks. try run linked in comment 12.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dbc3415780480c0c2065cf86f11d6a10ad55ed89&filter-searchStr=android

Is that not sufficient?
missed that, all looks good!
(Assignee)

Comment 32

2 years ago
Filed Bug 1358271
Whiteboard: [leave open]
(Assignee)

Comment 33

2 years ago
updated beta version of patch.
Attachment #8860025 - Attachment is obsolete: true
Attachment #8860164 - Flags: review+
(Assignee)

Comment 34

2 years ago
updated release version of patch.
Attachment #8860027 - Attachment is obsolete: true
Attachment #8860165 - Flags: review+

Comment 35

2 years ago
Pushed by bclary@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/68842e58a066
sync autophone webrtc test manifests with normal webrtc manifests, r=jmaher,dminor.
https://hg.mozilla.org/integration/mozilla-inbound/rev/0215b10785da
remove autophone webrtc test manifests, r=dminor.
(Assignee)

Comment 37

2 years ago
Update trychooser.

We have a bit of a race between landing this on inbound, merging to autoland and mozilla-central then uplifting to beta, mozila-release and landing the corresponding changes on Autophone.

We can probably live with the breakage if this goes live before everything is completed. I'll just deal with any try pushes that use the old syntax.
Attachment #8860174 - Flags: review?(dustin)
(Assignee)

Updated

2 years ago
See Also: → bug 1358030
Comment on attachment 8860174 [details] [review]
https://github.com/mozilla-releng/services/pull/269

I don't think I'm a great person to review that.  Reading through the bug I don't really know what's going on.

Github suggest garbas - I'm not sure he'd have an opinion either, but he can at least rubber-stamp.
Attachment #8860174 - Flags: review?(dustin)
(Assignee)

Updated

2 years ago
Attachment #8860174 - Flags: review?(rgarbas)
(Assignee)

Comment 40

2 years ago
Tomcat, can you take care of uplifting the appropriate beta and release versions of the patches today? There shouldn't be any merge conflicts with these patches unlike the trunk versions.

I'm intentionally skipping aurora though the beta patch applies there.
Flags: needinfo?(cbook)
Whiteboard: [leave open] → [leave open][checkin-needed-release][checkin-needed-beta]
hey bc, seems this needs rebasing for beta. could you take a look ? thanks!
Flags: needinfo?(cbook) → needinfo?(bob)
Flags: needinfo?(bob)
Whiteboard: [leave open][checkin-needed-release][checkin-needed-beta] → [leave open]
(Assignee)

Comment 44

2 years ago
https://github.com/mozilla/autophone/commit/b2c0b2fe0ff2405ddc469281dda993b0d93be7ee

I made two errors that required patches

https://github.com/mozilla/autophone/commit/ebec9c29cda6da6ec80c1db188aabf6b06301b17
Bug 1352333 - Autophone - fix duplicate devices in tests/production-autophone-2.ini, r=self

https://github.com/mozilla/autophone/commit/9703f5ed3f6ae73c796e8111269fdf83af208abe
Bug 1352333 - Autophone - mark nexus-6p-13 as a spare, r=self.
Comment hidden (Intermittent Failures Robot)
(Assignee)

Updated

2 years ago
Depends on: 1358827
Whiteboard: [leave open] → [leave open][stockwell fixed]
Comment hidden (Intermittent Failures Robot)
(Assignee)

Comment 47

2 years ago
(In reply to OrangeFactor Robot from comment #46)
> 71 failures in 817 pushes (0.087 failures/push) were associated with this
> bug in the last 7 days. 

This high failure rate was due to the failure on my part of coordinating the changes to autophone and the various repositories. I pointed the failures here to track them but this should not reoccur.
(Assignee)

Comment 48

2 years ago
Comment on attachment 8860174 [details] [review]
https://github.com/mozilla-releng/services/pull/269

garbas merged this over the weekend.
Attachment #8860174 - Flags: review?(rgarbas)
(Assignee)

Comment 49

2 years ago
This is a follow up to fix a copy/paste error on my part in the trychooser.
Attachment #8860802 - Flags: review?(rgarbas)
(Assignee)

Comment 50

2 years ago
https://mozilla-releng.net/trychooser/ is live.

Thanks garbas and everyone!
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
(Assignee)

Updated

2 years ago
Attachment #8860802 - Flags: review?(rgarbas)
Comment hidden (Intermittent Failures Robot)
You need to log in before you can comment on or make changes to this bug.