1240120 - external-media-tests files needs to be put in a subdirectory for correct packaging.

Status: RESOLVED FIXED

(Testing Graveyard :: external-media-tests, defect)

Description

[Syd Polk :sydpolk]

Right now, when mozharness sets up a venv, the directories in dom/media/test/external-media-tests are put at the top-level of the site-packages directory:

Syd-Polks-Mozilla-MBP:site-packages spolk$ ls
_markerlib				   media_utils			   mozprofile-0.27-py2.7.egg-info
blessings				   mozInstall-1.12-py2.7.egg-info  mozrunner
blessings-1.5.1-py2.7.egg-info		   mozcrash			   mozrunner-6.9-py2.7.egg-info
blobuploader				   mozcrash-0.16-py2.7.egg-info    mozscreenshot
blobuploader-1.2.4-py2.7.egg-info	   mozdebug			   mozscreenshot-0.1-py2.7.egg-info
browsermob_proxy-0.6.0-py2.7.egg-info	   mozdebug-0.1-py2.7.egg-info	   mozsystemmonitor
browsermobproxy				   mozdevice			   mozsystemmonitor-0.0-py2.7.egg-info
docopt-0.6.1-py2.7.egg-info		   mozdevice-0.46-py2.7.egg-info   moztest
docopt.py				   mozfile			   moztest-0.7-py2.7.egg-info
easy_install.py				   mozfile-1.2-py2.7.egg-info	   mozversion
easy_install.pyc			   mozhttpd			   mozversion-1.4-py2.7.egg-info
firefox_media_tests-0.4-py2.7.egg-info	   mozhttpd-0.7-py2.7.egg-info	   pip
firefox_puppeteer			   mozinfo			   pip-1.5.6.dist-info
firefox_puppeteer-3.1.0-py2.7.egg-info	   mozinfo-0.8-py2.7.egg-info	   pkg_resources.py
harness					   mozinstall			   pkg_resources.pyc
manifestparser				   mozleak			   psutil
manifestparser-1.1-py2.7.egg-info	   mozleak-0.1-py2.7.egg-info	   psutil-3.1.1-py2.7.egg-info
marionette				   mozlog			   requests
marionette_client-2.0.0-py2.7.egg-info	   mozlog-3.0-py2.7.egg-info	   requests-1.2.3-py2.7.egg-info
marionette_driver			   moznetwork			   setuptools
marionette_driver-1.1.1-py2.7.egg-info	   moznetwork-0.27-py2.7.egg-info  setuptools-3.6.dist-info
marionette_transport			   mozprocess			   wptserve
marionette_transport-1.0.0-py2.7.egg-info  mozprocess-0.22-py2.7.egg-info  wptserve-1.3.0-py2.7.egg-info
media_tests				   mozprofile
Syd-Polks-Mozilla-MBP:site-packages spolk$ pwd
/Users/spolk/dev/testbuildbot/build/venv/lib/python2.7/site-packages
Syd-Polks-Mozilla-MBP:site-packages spolk$ 


Notice the "harness", "media_tests", and "media_utils" directories.

Comment 1

[Syd Polk :sydpolk]

Attachment: MozReview Request: Bug 1240120 - Move external-media-tests to subdirectory to generate correct packaging; r?maja_zf, r?whimboo

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

Comment 2

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8709520 [details]
MozReview Request: Bug 1240120 - Move external-media-tests to subdirectory to generate correct packaging; r?maja_zf, r?whimboo

https://reviewboard.mozilla.org/r/31463/#review28215

The package name in setup.py (firefox-media-tests) should match the folder name you choose. So you might want dom/media/test/external-media-tests/firefox-media-tests

Comment 3

[Syd Polk :sydpolk]

"external-media-tests" is the name the media team wanted. Changing setup.py to match.

Comment 4

[Syd Polk :sydpolk]

Comment on attachment 8709520 [details]
MozReview Request: Bug 1240120 - Move external-media-tests to subdirectory to generate correct packaging; r?maja_zf, r?whimboo

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

Comment 5

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8709520 [details]
MozReview Request: Bug 1240120 - Move external-media-tests to subdirectory to generate correct packaging; r?maja_zf, r?whimboo

https://reviewboard.mozilla.org/r/31463/#review28257

I just noticed that MANIFEST.in needs to be updated to refer to the 'media_tests' dir that was previously renamed. Otherwise, resources like the default urls don't get included in the package.

Comment 6

[Syd Polk :sydpolk]

Comment on attachment 8709520 [details]
MozReview Request: Bug 1240120 - Move external-media-tests to subdirectory to generate correct packaging; r?maja_zf, r?whimboo

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

Comment 7

[Henrik Skupin (:whimboo) [⌚️UTC+1]]

Comment on attachment 8709520 [details]
MozReview Request: Bug 1240120 - Move external-media-tests to subdirectory to generate correct packaging; r?maja_zf, r?whimboo

https://reviewboard.mozilla.org/r/31463/#review28293

I think the move is fine but maybe you wanna talk with the media team again for a better folder naming. IMO dom/media/test/external/external-media-tests/ might make more sense. Otherwise lots of duplication in the names.

Comment 8

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8709520 [details]
MozReview Request: Bug 1240120 - Move external-media-tests to subdirectory to generate correct packaging; r?maja_zf, r?whimboo

https://reviewboard.mozilla.org/r/31463/#review28333

Clearing review flag until next round of changes comes in.

Comment 9

[Syd Polk :sydpolk]

Comment on attachment 8709520 [details]
MozReview Request: Bug 1240120 - Move external-media-tests to subdirectory to generate correct packaging; r?maja_zf, r?whimboo

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

Comment 10

[Syd Polk :sydpolk]

Comment on attachment 8709520 [details]
MozReview Request: Bug 1240120 - Move external-media-tests to subdirectory to generate correct packaging; r?maja_zf, r?whimboo

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31463/diff/4-5/

Comment 11

[Syd Polk :sydpolk]

Comment on attachment 8709520 [details]
MozReview Request: Bug 1240120 - Move external-media-tests to subdirectory to generate correct packaging; r?maja_zf, r?whimboo

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31463/diff/5-6/

Comment 12

[Syd Polk :sydpolk]

The package itself is now called "external-media-tests"; the name "firefox-media-tests" is gone. When you run "python setup.py", you will get a binary named "external-media-tests" now.

Comment 13

[Syd Polk :sydpolk]

Comment on attachment 8709520 [details]
MozReview Request: Bug 1240120 - Move external-media-tests to subdirectory to generate correct packaging; r?maja_zf, r?whimboo

The current packaging does not work with mozharness. Need to talk about this on #irc.

Comment 14

[Syd Polk :sydpolk]

Comment on attachment 8709520 [details]
MozReview Request: Bug 1240120 - Move external-media-tests to subdirectory to generate correct packaging; r?maja_zf, r?whimboo

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31463/diff/6-7/

Comment 15

[Syd Polk :sydpolk]

Comment on attachment 8709520 [details]
MozReview Request: Bug 1240120 - Move external-media-tests to subdirectory to generate correct packaging; r?maja_zf, r?whimboo

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/31463/diff/7-8/

Comment 16

[Syd Polk :sydpolk]

This is the first of three patches that are chained to each other. This one sets the directory structure to the following:

dom/media/test/external:
MANIFEST.in  external_media_harness  requirements.txt
README.md    external_media_tests    setup.py

This runs fine, but with invoking directly with external_media_harness/runtests.py, and with external-media-tests after running python setup.py develop.

Comment 17

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Test it locally with setup.py install, otherwise you won't catch missing resources in the package.

Comment 18

[Maja Frydrychowicz :impossibus (was :maja_zf) (needinfo me)]

Comment on attachment 8709520 [details]
MozReview Request: Bug 1240120 - Move external-media-tests to subdirectory to generate correct packaging; r?maja_zf, r?whimboo

https://reviewboard.mozilla.org/r/31463/#review28695

Looks good.

::: dom/media/test/external/MANIFEST.in:2
(Diff revision 8)
> -include requirements.txt
> +recursive-include external_media_harness *

This line is not needed, you only need the extra files from external_media_tests. It doesn't hurt to keep it, though.

Also, do you want to include requirements.txt as well?

Comment 19

[Syd Polk :sydpolk]

https://reviewboard.mozilla.org/r/31463/#review28695

> This line is not needed, you only need the extra files from external_media_tests. It doesn't hurt to keep it, though.
> 
> Also, do you want to include requirements.txt as well?

There is no place in the site-packages for the requirements.txt to go. It gets in to the common.tests.zip just fine.

Comment 20

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5d8658bcefdc

Comment 21

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/5d8658bcefdc