Closed Bug 1240120 Opened 5 years ago Closed 5 years ago

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

Categories

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

Product:
Testing Graveyard
Component:
external-media-tests
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox46 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla46
Tracking Flags:
Tracking Status
firefox46 --- fixed

People

(Reporter: sydpolk, Assigned: sydpolk)

References

Details

Attachments

(1 file)

MozReview Request: Bug 1240120 - Move external-media-tests to subdirectory to generate correct packaging; r?maja_zf, r?whimboo
58 bytes, text/x-review-board-request
whimboo
: review+
impossibus
: review+
Details 
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 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
Attachment #8709520 - Flags: review?(mjzffr)
"external-media-tests" is the name the media team wanted. Changing setup.py to match.
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/
Attachment #8709520 - Attachment description: MozReview Request: Bug 1240120 - Move external media tests into a subdirectory for correct packaging. r?whimboo, r?maja_zf → MozReview Request: Bug 1240120 - Move external-media-tests to subdirectory to generate correct packaging; r?maja_zf, r?whimboo
Attachment #8709520 - Flags: review?(mjzffr)
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 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/
Attachment #8709520 - Flags: review?(mjzffr)
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.
Attachment #8709520 - Flags: review?(hskupin) → review+
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.
Attachment #8709520 - Flags: review?(mjzffr)
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/
Attachment #8709520 - Flags: review?(mjzffr)
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 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/
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.
Blocks: 1240991
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.
Attachment #8709520 - Flags: review?(mjzffr) → review-
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/
Attachment #8709520 - Flags: review- → review?(mjzffr)
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/
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.
Test it locally with setup.py install, otherwise you won't catch missing resources in the package.
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?
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.
https://hg.mozilla.org/mozilla-central/rev/5d8658bcefdc
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.