Closed Bug 1209327 Opened 4 years ago Closed 4 years ago

schedule media-youtube-tests on cedar

Categories

(Release Engineering :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jgriffin, Assigned: jgriffin)

References

Details

Attachments

(4 files, 1 obsolete file)

These are analogs to the media-tests from bug 1209258.
According to maja, we want to invoke these tests using:

scripts/scripts/firefox_media_tests_buildbot.py --cfg mediatests/buildbot_posix_config.py --blob-upload-branch cedar --download-symbols ondemand --tests /builds/slave/test/build/firefox-media-tests/firefox_media_tests/playback/youtube/manifest.ini

In order to do this, we'll need to introduce an in-tree config for this.
Bug 1209327 - Add in-tree config for mediatests, r=maja
Attachment #8671079 - Flags: review?(mjzffr)
Comment on attachment 8671079 [details]
MozReview Request: Bug 1209327 - Add in-tree config for mediatests, r=maja

https://reviewboard.mozilla.org/r/21529/#review19405

::: testing/mozharness/mozharness/mozilla/testing/firefox_media_tests.py:55
(Diff revision 1)
> +    [["--youtube"],

Instead of `--youtube`, what about having a `--suite` option with default value `media-tests`? That way we can add other suites (e.g. EME tests) without adding more options. I think it would simplify some of the logic in `_query_cmd()` as well.

::: testing/mozharness/mozharness/mozilla/testing/firefox_media_tests.py:234
(Diff revision 1)
> +

I think all these changes fit better in `mozharness/scripts/firefox_media_tests_buildbot.py`.
Attachment #8671079 - Flags: review?(mjzffr)
Attachment #8671079 - Flags: review?(mjzffr)
Comment on attachment 8671079 [details]
MozReview Request: Bug 1209327 - Add in-tree config for mediatests, r=maja

Bug 1209327 - Add in-tree config for mediatests, r=maja
(In reply to Maja Frydrychowicz (:maja_zf) from comment #4)
> Comment on attachment 8671079 [details]
> MozReview Request: Bug 1209327 - Add in-tree config for mediatests, r=maja
> 
> https://reviewboard.mozilla.org/r/21529/#review19405
> 
> ::: testing/mozharness/mozharness/mozilla/testing/firefox_media_tests.py:55
> (Diff revision 1)
> > +    [["--youtube"],
> 
> Instead of `--youtube`, what about having a `--suite` option with default
> value `media-tests`? That way we can add other suites (e.g. EME tests)
> without adding more options. I think it would simplify some of the logic in
> `_query_cmd()` as well.

good idea...done!

> 
> ::: testing/mozharness/mozharness/mozilla/testing/firefox_media_tests.py:234
> (Diff revision 1)
> > +
> 
> I think all these changes fit better in
> `mozharness/scripts/firefox_media_tests_buildbot.py`.

yes, good call; I forgot this was shared between buildbot and jenkins
Comment on attachment 8671079 [details]
MozReview Request: Bug 1209327 - Add in-tree config for mediatests, r=maja

https://reviewboard.mozilla.org/r/21529/#review19459

Looks good, thanks. One optional change.

::: testing/mozharness/mozharness/mozilla/testing/firefox_media_tests.py:55
(Diff revision 2)
> +    [["--suite"],
> +     {"action": "store",
> +      "dest": "test_suite",
> +      "default": "media-tests",
> +      "help": "suite name",
> +      }],

Nit: This is only useful for buildbot, so I'd prefer it to be in  `firefox_media_tests_buildbot.py` as well.
Attachment #8671079 - Flags: review?(mjzffr) → review+
Keywords: leave-open
Builders added:
+ Rev5 MacOSX Yosemite 10.10 cedar debug test media-youtube-tests
+ Rev5 MacOSX Yosemite 10.10 cedar opt test media-youtube-tests
+ Ubuntu ASAN VM 12.04 x64 cedar opt test media-youtube-tests
+ Ubuntu VM 12.04 x64 cedar debug test media-youtube-tests
+ Ubuntu VM 12.04 x64 cedar opt test media-youtube-tests
+ Windows 7 32-bit cedar debug test media-youtube-tests
+ Windows 7 32-bit cedar opt test media-youtube-tests
+ Windows 8 64-bit cedar debug test media-youtube-tests
+ Windows 8 64-bit cedar opt test media-youtube-tests
Comment on attachment 8672122 [details] [diff] [review]
Schedule media-youtube-tests on cedar,

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

lgtm. sorry for the delay. Canadia had a long weekend
Attachment #8672122 - Flags: review?(jlund) → review+
Forgot this little detail...
Attachment #8675814 - Flags: review?(jlund)
Comment on attachment 8675814 [details] [diff] [review]
specify config file for media-youtube-tests,

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

:)
Attachment #8675814 - Flags: review?(jlund) → review+
Comment on attachment 8675814 [details] [diff] [review]
specify config file for media-youtube-tests,

https://hg.mozilla.org/build/buildbot-configs/rev/37e1d31fb11b
Attachment #8675814 - Flags: checked-in+
Comment on attachment 8671079 [details]
MozReview Request: Bug 1209327 - Add in-tree config for mediatests, r=maja

https://reviewboard.mozilla.org/r/21529/#review21301

::: testing/mozharness/configs/mediatests/buildbot_windows_config.py:66
(Diff revision 2)
> +                "--tests=%(test_manifest)s"

Ah, I missed this earlier: this should just be `%(test_manifest)s`. 

When I described using a `--tests` option earlier, I have talking about the mozharness script for firefox-media-tests. `runtests.py`, which is called in `_query_cmd`, has no `--tests` flag.
Attachment #8671079 - Flags: review+
Attachment #8681943 - Flags: review?(mjzffr)
Attachment #8681943 - Attachment is obsolete: true
Attachment #8681943 - Flags: review?(mjzffr)
Attachment #8681945 - Flags: review?(mjzffr) → review+
These are green now; I'll file a separate bug to enable them on trunk.
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Blocks: 1221963
removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.