We got burned in bug 1058073 by some incomplete downloads from tooltool because we weren't using the actual tooltool.py (via tooltool_fetch) which provides built-in integrity checking. There's a shim in place with https://hg.mozilla.org/build/mozharness/rev/757022a9aa45, but we should really switch to using tooltool_fetch for downloads.

(In reply to Chris Cooper [:coop] from comment #0) > There's a shim in place with > https://hg.mozilla.org/build/mozharness/rev/757022a9aa45, but we should > really switch to using tooltool_fetch for downloads. Should also point to https://hg.mozilla.org/build/mozharness/rev/916fd25c7692 which has bulk of the non-bustage fix work.

One problem here (and likely why this wasn't implemented properly in the first place) is that we don't currently use manifests for tests. I poked around the mozharness code to verify this, but do enlighten me if I'm wrong. The tooltool manifests for builds are in the source trees, and we really don't want to start sucking down entire source trees for tests. We really do want any test manifests to live with the source so they can easily ride the trains. Provided we've already run the 'read-buildbot-config' step and have a revision, there's no reason we couldn't use the branch, etc to grab a single file, i.e. a test manifest, from the relevant source tree.

Two ideas about how to proceed: 1) Currently the details about artifacts which need to be fetched from tooltool server (filename, sha512 sum, local cache folder) are stored in mozharness configuration files (configs/android/androidarm.py and configs/android/androidx86.py). It would be possible to keep the tooltool information there and follow the same approach used in http://hg.mozilla.org/build/mozharness/file/6353da5601d6/mozharness/mozilla/testing/unittest.py#l205. Basically a manifest would be materialized after reading its content from the configuration, and then used for tooltool calls. 2) If we want the manifests to live with the source (as coop proposes), we could do something similar to http://hg.mozilla.org/build/mozharness/file/6353da5601d6/mozharness/mozilla/testing/unittest.py#l210

Hi Armen, I chose you for review since you originally defined the download_cacheable_artifacts method, which has now been removed. Please note that: - Tooltool is now able to manage a local cache (https://wiki.mozilla.org/ReleaseEngineering/Tooltool#The_tooltool_cache) - Tooltool has an integrated unpacking mechanism for archive files (see Bug 988785)

Comment on attachment 8495853 [details] [diff] [review] Use tooltool to fetch avd tarballs, manage a local cache and unpack archives. Review of attachment 8495853 [details] [diff] [review]: ----------------------------------------------------------------- Hi Simone! This looks good. However, I'm setting r- mainly because I want the manifest in the tree. Thank you for fixing this! ::: scripts/android_emulator_unittest.py @@ +470,3 @@ > ''' > c = self.config > self.rmtree(c[".avds_dir"]) I should have added a note in here. Would you mind saying that we would not need to unpack if we had a way to know in the unpacked contents match the contents of the tar ball?

Thanks Armen! > However, I'm setting r- mainly because I want the manifest in the tree. Ok! I put the manifest content in the same place where the previous configuration relative to these artifacts was, but if in-tree manifests are more convenient, it's ok for me too . Besides, that should be a pretty simple change. What would be a suitable in-tree location for these two manifests? > ::: scripts/android_emulator_unittest.py > @@ +470,3 @@ > > ''' > > c = self.config > > self.rmtree(c[".avds_dir"]) > > I should have added a note in here. > Would you mind saying that we would not need to unpack if we had a way to > know in the unpacked contents match the contents of the tar ball? I am not sure whether I understand this remark. The tooltool fetch command (with cache enabled) works as follows: 1) when the fetch command is issued, tooltool looks for the unpacked artifact in the cache; 2a) if the artifact is already there, it does not fetch it from the tooltool server but from the cache, then it unpacks it in the specified output folder 2b) if it is not in the cache, it fetches it from the tooltool server, stores it in the cache for future use, and unpacks it In all cases, checksum verifications are performed to make sure that there was no file corruption while fetching the archive from the tooltool server or moving it from the cache. In my understanding this is exactly what is needed here, so no extra checks or extra logic is needed. Does this address your note? If not, please let me know. Thanks again!

Comment on attachment 8495853 [details] [diff] [review] Use tooltool to fetch avd tarballs, manage a local cache and unpack archives. Review of attachment 8495853 [details] [diff] [review]: ----------------------------------------------------------------- I forgot to set the flag.

(In reply to Simone Bruno [:simone] from comment #6) > Thanks Armen! > > > However, I'm setting r- mainly because I want the manifest in the tree. > > Ok! I put the manifest content in the same place where the previous > configuration relative to these artifacts was, but if in-tree manifests are > more convenient, it's ok for me too . > Besides, that should be a pretty simple change. > > What would be a suitable in-tree location for these two manifests? > I have asked in the #ateam channel and we believe it should be in here: testing/config/tooltool-manifests/$platform/releng.manifest > > ::: scripts/android_emulator_unittest.py > > @@ +470,3 @@ > > > ''' > > > c = self.config > > > self.rmtree(c[".avds_dir"]) > > > > I should have added a note in here. > > Would you mind saying that we would not need to unpack if we had a way to > > know in the unpacked contents match the contents of the tar ball? > > I am not sure whether I understand this remark. The tooltool fetch command > (with cache enabled) works as follows: > 1) when the fetch command is issued, tooltool looks for the unpacked > artifact in the cache; Did you add "unpacked" in that sentence by mistake? If not, could you please point me to the right code? IIUC, tooltool only checks that the downloaded file has the same shasum as in the manifest. e.g. in http://hg.mozilla.org/mozilla-central/file/default/b2g/config/tooltool-manifests/linux32/releng.manifest It will only check that sccache.tar.bz has been downloaded, no? I don't think it verifies the contents that come out after extraction. I could be wrong though! If it does, then we won't have to clobber the .avds_dir every time and won't need such a comment. > 2a) if the artifact is already there, it does not fetch it from the tooltool > server but from the cache, then it unpacks it in the specified output folder > 2b) if it is not in the cache, it fetches it from the tooltool server, > stores it in the cache for future use, and unpacks it > In all cases, checksum verifications are performed to make sure that there > was no file corruption while fetching the archive from the tooltool server > or moving it from the cache. > In my understanding this is exactly what is needed here, so no extra checks > or extra logic is needed. > Does this address your note? If not, please let me know. > > Thanks again!

> > 1) when the fetch command is issued, tooltool looks for the unpacked > > artifact in the cache; > > Did you add "unpacked" in that sentence by mistake? If not, could you please > point me to the right code? Whoops! You are right, wrong word choice (sorry!): tooltool checks the archive integrity before unpacking. > IIUC, tooltool only checks that the downloaded file has the same shasum as > in the manifest. Indeed! > If it does, then we won't have to clobber the .avds_dir every time and won't > need such a comment. Apparently we'll need the clobbering and I'll add the comment you suggested.

Comment on attachment 8496817 [details] [diff] [review] mozharness_v2 Review of attachment 8496817 [details] [diff] [review]: ----------------------------------------------------------------- This code is very well written and does what I asked for, however, I have overlooked something critical. Switching from reading this info from mozharness to in-tree is not trivial. We would break older branches. There are three solutions: 1) Lock mozharness from the tree 2) We would have to support both approaches, in-tree info and in-mozharness info 3) uplift your other patch to every branch that has Android test jobs The cleaner options is #3. You can land the patch on release branches with a=NPOTB and DONTBUILD in the commit message. Unfortunately this process requires checking out the repo for every branch you want to check into. You can also request others to land them for you, however, it can take some time [1] My apologies for not noticing this when requesting the reading of the info from the tree. I'm mainly r- because this would have broken the jobs in many branches. Could you also add some code to clear this directory? I don't want to leave stuff behind without being clobbered. I want to reduce unnecessary disk print. "tooltool_cache_path": "/builds/slave/talos-slave/cached" We can remove that piece of code on a week or so when all of our machines have cleared that cache. Thank you for taking care of this refactoring! It will pay off in the long term. [1] https://wiki.mozilla.org/Sheriffing/How:To:Landing_checkin-needed_patches ::: scripts/android_emulator_unittest.py @@ +470,5 @@ > ''' > c = self.config > + > + # clobbering and re-unpacking would not be needed if we had a way to check whether > + # the unpacked content already present match the contents of the tar ball Thanks! Please add FIXME keyword on the comment. It helps to see these areas of improvements with syntax highlighting that vim and other editors have. @@ +478,5 @@ > + url = 'https://hg.mozilla.org/%s/raw-file/%s/%s' % ( > + self.buildbot_config['repo_path'], self.buildbot_config['revision'], c["tooltool_manifest_path"]) > + manifest_path = self.download_file(url, > + file_name='releng.manifest', > + parent_dir=c[".avds_dir"]) For the record, you don't have to download the file. It is already inside of the tests.zip. manifest_path = os.path.join(self.abs_dir["abs_test_install_dir"], "config/tooltool-manifests/androidx86/releng.manifest") You can push your other patch to Ash (and backout it out right after with DONTBUILD) and test this with the Android test jobs. Not hitting hgweb will also be beneficial for HG load.

Comment on attachment 8496818 [details] [diff] [review] adding in-tree tooltoolmanifests https://hg.mozilla.org/integration/mozilla-inbound/rev/b89a700ab389 I will push the mozharness change when this will be propagated to other trees.

Thanks a lot for your feedback! I will agree with Sheriffs how to proceed to land this. I'll provide another mozharness patch to address armenzg's notes in comment 13, and land on Ash first to verify whether everything works as expected.

Comment on attachment 8497407 [details] [diff] [review] mozharness_v3 Review of attachment 8497407 [details] [diff] [review]: ----------------------------------------------------------------- It's almost there; some nits and adjusting some small paths. Please address the issues and test it on Ash to make sure that is does what we want. ::: scripts/android_emulator_unittest.py @@ +475,5 @@ > + # it can be removed when the folder has been clobbered on all slaves > + old_tooltool_cache="/builds/slave/talos-slave/cached" > + try: > + self.rmtree(old_tooltool_cache) > + self.info("Folder %s is no longer used to cache tooltool artifacts and has been deleted") It's missing % old_tooltool_cache. @@ +477,5 @@ > + try: > + self.rmtree(old_tooltool_cache) > + self.info("Folder %s is no longer used to cache tooltool artifacts and has been deleted") > + except OSError as e: > + self.info("Folder %s has not been clobbered: %s" % (old_tooltool_cache, e)) A Warning instead of an info. str(e) instead of e for the string substitution. @@ +478,5 @@ > + self.rmtree(old_tooltool_cache) > + self.info("Folder %s is no longer used to cache tooltool artifacts and has been deleted") > + except OSError as e: > + self.info("Folder %s has not been clobbered: %s" % (old_tooltool_cache, e)) > + Nit: white space @@ +488,4 @@ > self.mkdir_p(c[".avds_dir"]) > + > + manifest_path = os.path.join(self.abs_dir["abs_test_install_dir"], c["tooltool_manifest_path"]) > + Nit: white space @@ +488,5 @@ > self.mkdir_p(c[".avds_dir"]) > + > + manifest_path = os.path.join(self.abs_dir["abs_test_install_dir"], c["tooltool_manifest_path"]) > + > + if not manifest_path: This condition would not never be reached as os.path.join() will most likely never return None. I think you want to use os.path.exists(). That would check if the file is there. @@ +490,5 @@ > + manifest_path = os.path.join(self.abs_dir["abs_test_install_dir"], c["tooltool_manifest_path"]) > + > + if not manifest_path: > + self.fatal("Can't determine location of tooltool manifest needed to retrieve avds artifacts") > + if self.tooltool_fetch(manifest_path, output_dir=c[".avds_dir"], cache=c.get("tooltool_cache", None)): I don't think this does what you want. Unfortunately, that function does not return any information about how things went. [1] It *always* returns None (see example below). [1] http://hg.mozilla.org/build/mozharness/file/default/mozharness/mozilla/tooltool.py#l17 armenzg-thinkpad mozharness hg:[default!] $ cat test.py def my_func(): print "Hello" print my_func() armenzg-thinkpad mozharness hg:[default!] $ python test.py Hello None

Armen, thanks a lot for your patience!

Comment on attachment 8497577 [details] [diff] [review] mozharness_v4 Review of attachment 8497577 [details] [diff] [review]: ----------------------------------------------------------------- Excellent! BTW did you run this locally on Linux64 host? Let me know if Ash yields what you expect. Poke me if you want to coordinate the testing with chmanchester and me.

The manifests have been added to the moz central repo, but not to Aurora, Beta, Release, and ESR31 yet; furthermore but the mozharness part still needs to be landed. I'll do as soon as I complete the tests in my stage environment.

Adding [leave open] t the whiteboard when landing to m-i prevents sheriffs from closing the bug once merged to mozilla-central. I hope that helps!

First of all, my apologies for not having tested properly this patch before uploading the previous versions. This caused some avoidable waste of time both on armen's (as reviewer) and my side. This version of the mozharness patch has been tested in my staging environment. The output of the setup_avds step is: 09:18:49 INFO - ##### 09:18:49 INFO - ##### Running setup-avds step. 09:18:49 INFO - ##### 09:18:49 INFO - Running pre-action listener: _resource_record_pre_action 09:18:49 INFO - Running main action method: setup_avds 09:18:49 INFO - rmtree: /builds/slave/talos-slave/cached 09:18:49 INFO - Folder /builds/slave/talos-slave/cached is no longer used to cache tooltool artifacts and has been deleted 09:18:49 INFO - rmtree: /home/cltbld/.android 09:18:49 INFO - retry: Calling <function rmtree at 0x21c3a28> with args: ('/home/cltbld/.android',), kwargs: {}, attempt #1 09:18:49 INFO - mkdir: /home/cltbld/.android 09:18:49 INFO - Downloading https://hg.mozilla.org/mozilla-central/raw-file/9ee9e193fc48/testing/config/tooltool-manifests/androidarm/releng.manifest to /home/cltbld/.android/releng.manifest 09:18:49 INFO - retry: Calling <bound method AndroidEmulatorTest._download_file of <__main__.AndroidEmulatorTest object at 0x2464990>> with args: ('https://hg.mozilla.org/mozilla-central/raw-file/9ee9e193fc48/testing/config/tooltool-manifests/androidarm/releng.manifest', '/home/cltbld/.android/releng.manifest'), kwargs: {}, attempt #1 09:18:51 INFO - Downloaded 277 bytes. 09:18:51 INFO - proxxy config: {'regions': ['.use1.', '.usw2.'], 'instances': ['proxxy.srv.releng.use1.mozilla.com', 'proxxy.srv.releng.usw2.mozilla.com'], 'urls': [('http://ftp.mozilla.org', 'ftp.mozilla.org'), ('https://ftp.mozilla.org', 'ftp.mozilla.org'), ('https://ftp-ssl.mozilla.org', 'ftp.mozilla.org'), ('http://pvtbuilds.pvt.build.mozilla.org', 'pvtbuilds.mozilla.org'), ('http://runtime-binaries.pvt.build.mozilla.org', 'runtime-binaries.pvt.build.mozilla.org'), ('http://pypi.pvt.build.mozilla.org', 'pypi.pvt.build.mozilla.org'), ('http://pypi.pub.build.mozilla.org', 'pypi.pub.build.mozilla.org')]} 09:18:51 INFO - http://runtime-binaries.pvt.build.mozilla.org/tooltool/ matches http://runtime-binaries.pvt.build.mozilla.org 09:18:51 INFO - URL Candidate: http://runtime-binaries.pvt.build.mozilla.org.proxxy.srv.releng.use1.mozilla.com/tooltool/ 09:18:51 INFO - retry: Calling <bound method AndroidEmulatorTest.run_command of <__main__.AndroidEmulatorTest object at 0x2464990>> with args: (['/tools/tooltool.py', '--url', 'http://runtime-binaries.pvt.build.mozilla.org.proxxy.srv.releng.use1.mozilla.com/tooltool/', '--url', 'http://runtime-binaries.pvt.build.mozilla.org/tooltool/', 'fetch', '-m', '/home/cltbld/.android/releng.manifest', '-o', '-c', '/builds/tooltool_cache'],), kwargs: {'error_list': [{'substr': 'command not found', 'level': 'error'}, {'regex': <_sre.SRE_Pattern object at 0x2491340>, 'level': 'warning'}, {'substr': 'Traceback (most recent call last)', 'level': 'error'}, {'substr': 'SyntaxError: ', 'level': 'error'}, {'substr': 'TypeError: ', 'level': 'error'}, {'substr': 'NameError: ', 'level': 'error'}, {'substr': 'ZeroDivisionError: ', 'level': 'error'}, {'regex': <_sre.SRE_Pattern object at 0x240c950>, 'level': 'critical'}, {'regex': <_sre.SRE_Pattern object at 0x25158b0>, 'level': 'critical'}, {'substr': 'ERROR - ', 'level': 'error'}], 'cwd': '/home/cltbld/.android', 'privileged': False}, attempt #1 09:18:51 INFO - Running command: ['/tools/tooltool.py', '--url', 'http://runtime-binaries.pvt.build.mozilla.org.proxxy.srv.releng.use1.mozilla.com/tooltool/', '--url', 'http://runtime-binaries.pvt.build.mozilla.org/tooltool/', 'fetch', '-m', '/home/cltbld/.android/releng.manifest', '-o', '-c', '/builds/tooltool_cache'] in /home/cltbld/.android 09:18:51 INFO - Copy/paste: /tools/tooltool.py --url http://runtime-binaries.pvt.build.mozilla.org.proxxy.srv.releng.use1.mozilla.com/tooltool/ --url http://runtime-binaries.pvt.build.mozilla.org/tooltool/ fetch -m /home/cltbld/.android/releng.manifest -o -c /builds/tooltool_cache 09:18:51 INFO - INFO - File AVDs-armv7a-gingerbread-build-2014-01-23-ubuntu.tar.gz not present in local cache folder /builds/tooltool_cache 09:18:51 INFO - INFO - Attempting to fetch from 'http://runtime-binaries.pvt.build.mozilla.org.proxxy.srv.releng.use1.mozilla.com/tooltool/'... 09:18:55 INFO - INFO - File AVDs-armv7a-gingerbread-build-2014-01-23-ubuntu.tar.gz fetched from http://runtime-binaries.pvt.build.mozilla.org.proxxy.srv.releng.use1.mozilla.com/tooltool/ as /home/cltbld/.android/tmp8bsFe3 09:18:57 INFO - INFO - File integrity verified, renaming tmp8bsFe3 to AVDs-armv7a-gingerbread-build-2014-01-23-ubuntu.tar.gz 09:18:57 INFO - INFO - Updating local cache /builds/tooltool_cache... 09:19:02 INFO - INFO - Local cache /builds/tooltool_cache updated with AVDs-armv7a-gingerbread-build-2014-01-23-ubuntu.tar.gz 09:19:02 INFO - INFO - untarring "AVDs-armv7a-gingerbread-build-2014-01-23-ubuntu.tar.gz" 09:20:30 INFO - Return code: 0 09:20:30 INFO - Running post-action listener: _resource_record_post_action 09:20:30 INFO - ##### 09:20:30 INFO - ##### Running start-emulators step. When running the same task again, the output is as follows (as expected, the artifacts after the first run are taken from the tooltool_cache): 10:27:42 INFO - ##### 10:27:42 INFO - ##### Running setup-avds step. 10:27:42 INFO - ##### 10:27:42 INFO - Running pre-action listener: _resource_record_pre_action 10:27:42 INFO - Running main action method: setup_avds 10:27:42 INFO - rmtree: /builds/slave/talos-slave/cached 10:27:42 INFO - Folder /builds/slave/talos-slave/cached is no longer used to cache tooltool artifacts and has been deleted 10:27:42 INFO - rmtree: /home/cltbld/.android 10:27:42 INFO - retry: Calling <function rmtree at 0x2140a28> with args: ('/home/cltbld/.android',), kwargs: {}, attempt #1 10:27:42 INFO - mkdir: /home/cltbld/.android 10:27:42 INFO - Downloading https://hg.mozilla.org/mozilla-central/raw-file/9ee9e193fc48/testing/config/tooltool-manifests/androidarm/releng.manifest to /home/cltbld/.android/releng.manifest 10:27:42 INFO - retry: Calling <bound method AndroidEmulatorTest._download_file of <__main__.AndroidEmulatorTest object at 0x23e2990>> with args: ('https://hg.mozilla.org/mozilla-central/raw-file/9ee9e193fc48/testing/config/tooltool-manifests/androidarm/releng.manifest', '/home/cltbld/.android/releng.manifest'), kwargs: {}, attempt #1 10:27:43 INFO - Downloaded 277 bytes. 10:27:43 INFO - proxxy config: {'regions': ['.use1.', '.usw2.'], 'instances': ['proxxy.srv.releng.use1.mozilla.com', 'proxxy.srv.releng.usw2.mozilla.com'], 'urls': [('http://ftp.mozilla.org', 'ftp.mozilla.org'), ('https://ftp.mozilla.org', 'ftp.mozilla.org'), ('https://ftp-ssl.mozilla.org', 'ftp.mozilla.org'), ('http://pvtbuilds.pvt.build.mozilla.org', 'pvtbuilds.mozilla.org'), ('http://runtime-binaries.pvt.build.mozilla.org', 'runtime-binaries.pvt.build.mozilla.org'), ('http://pypi.pvt.build.mozilla.org', 'pypi.pvt.build.mozilla.org'), ('http://pypi.pub.build.mozilla.org', 'pypi.pub.build.mozilla.org')]} 10:27:43 INFO - http://runtime-binaries.pvt.build.mozilla.org/tooltool/ matches http://runtime-binaries.pvt.build.mozilla.org 10:27:43 INFO - URL Candidate: http://runtime-binaries.pvt.build.mozilla.org.proxxy.srv.releng.use1.mozilla.com/tooltool/ 10:27:43 INFO - retry: Calling <bound method AndroidEmulatorTest.run_command of <__main__.AndroidEmulatorTest object at 0x23e2990>> with args: (['/tools/tooltool.py', '--url', 'http://runtime-binaries.pvt.build.mozilla.org.proxxy.srv.releng.use1.mozilla.com/tooltool/', '--url', 'http://runtime-binaries.pvt.build.mozilla.org/tooltool/', 'fetch', '-m', '/home/cltbld/.android/releng.manifest', '-o', '-c', '/builds/tooltool_cache'],), kwargs: {'error_list': [{'substr': 'command not found', 'level': 'error'}, {'regex': <_sre.SRE_Pattern object at 0x2354e00>, 'level': 'warning'}, {'substr': 'Traceback (most recent call last)', 'level': 'error'}, {'substr': 'SyntaxError: ', 'level': 'error'}, {'substr': 'TypeError: ', 'level': 'error'}, {'substr': 'NameError: ', 'level': 'error'}, {'substr': 'ZeroDivisionError: ', 'level': 'error'}, {'regex': <_sre.SRE_Pattern object at 0x2389f30>, 'level': 'critical'}, {'regex': <_sre.SRE_Pattern object at 0x237e0a0>, 'level': 'critical'}, {'substr': 'ERROR - ', 'level': 'error'}], 'cwd': '/home/cltbld/.android', 'privileged': False}, attempt #1 10:27:43 INFO - Running command: ['/tools/tooltool.py', '--url', 'http://runtime-binaries.pvt.build.mozilla.org.proxxy.srv.releng.use1.mozilla.com/tooltool/', '--url', 'http://runtime-binaries.pvt.build.mozilla.org/tooltool/', 'fetch', '-m', '/home/cltbld/.android/releng.manifest', '-o', '-c', '/builds/tooltool_cache'] in /home/cltbld/.android 10:27:43 INFO - Copy/paste: /tools/tooltool.py --url http://runtime-binaries.pvt.build.mozilla.org.proxxy.srv.releng.use1.mozilla.com/tooltool/ --url http://runtime-binaries.pvt.build.mozilla.org/tooltool/ fetch -m /home/cltbld/.android/releng.manifest -o -c /builds/tooltool_cache 10:27:47 INFO - INFO - File AVDs-armv7a-gingerbread-build-2014-01-23-ubuntu.tar.gz retrieved from local cache /builds/tooltool_cache 10:27:49 INFO - INFO - untarring "AVDs-armv7a-gingerbread-build-2014-01-23-ubuntu.tar.gz" 10:29:03 INFO - Return code: 0 10:29:03 INFO - Running post-action listener: _resource_record_post_action I also verified that the untarring is performed correctly, and the next execution steps are executed successfully. The showed output excerpts are taken from http://dev-master1.srv.releng.scl3.mozilla.com:8401/builders/Android%202.3%20Emulator%20mozilla-central%20opt%20test%20mochitest-gl/builds/7 and http://dev-master1.srv.releng.scl3.mozilla.com:8401/builders/Android%202.3%20Emulator%20mozilla-central%20opt%20test%20mochitest-gl/builds/8 respectively (these links will not be active for long). A final note for armenzg: I was not able to use a local version of the required releng manifest on the slave, so this version of the patch retrieves it from its in-tree location (in this case: https://hg.mozilla.org/mozilla-central/raw-file/9ee9e193fc48/testing/config/tooltool-manifests/androidarm/releng.manifest to /home/cltbld/.android/releng.manifest).

Comment on attachment 8501209 [details] [diff] [review] mozharness_v5 Review of attachment 8501209 [details] [diff] [review]: ----------------------------------------------------------------- This does the job. Thanks for your hard work. Could you please file a follow up bug for the following? In the A-team we're trying to move towards having everything we need within the test.zip. We want to reduce how much we hit hg web and working towards making the tests.zip stand-alone. > A final note for armenzg: I was not able to use a local version of the > required releng manifest on the slave, so this version of the patch > retrieves it from its in-tree location (in this case: > https://hg.mozilla.org/mozilla-central/raw-file/9ee9e193fc48/testing/config/ > tooltool-manifests/androidarm/releng.manifest to > /home/cltbld/.android/releng.manifest). ::: scripts/android_emulator_unittest.py @@ +488,3 @@ > self.mkdir_p(c[".avds_dir"]) > + > + url = 'https://hg.mozilla.org/%s/raw-file/%s/%s' % (self.buildbot_config['properties']['repo_path'], self.buildbot_config['properties']['revision'], c["tooltool_manifest_path"]) Nit: Could you please break this into two lines? @@ +488,4 @@ > self.mkdir_p(c[".avds_dir"]) > + > + url = 'https://hg.mozilla.org/%s/raw-file/%s/%s' % (self.buildbot_config['properties']['repo_path'], self.buildbot_config['properties']['revision'], c["tooltool_manifest_path"]) > + Nit: white spaces.

Comment on attachment 8501209 [details] [diff] [review] mozharness_v5 Hi Jordan, in this patch we add the ability of using tooltool for test jobs. I don't need a full review for you, however, I would you like to have a quick look to see if it makes sense to you.

Thanks Armen! Waiting for Jordan's feedback before landing.

(In reply to Simone Bruno [:simone] from comment #25) > Thanks Armen! Waiting for Jordan's feedback before landing. Feel free to land. It is a safe patch, however, I want jlund to be aware of this new functionality. If any refactoring is needed he will let you know and we can follow up.

Comment on attachment 8501209 [details] [diff] [review] mozharness_v5 Review of attachment 8501209 [details] [diff] [review]: ----------------------------------------------------------------- lgtm! this should be helpful. I've been asked a lot about tooltool and android lately. ::: scripts/android_emulator_unittest.py @@ +488,3 @@ > self.mkdir_p(c[".avds_dir"]) > + > + url = 'https://hg.mozilla.org/%s/raw-file/%s/%s' % (self.buildbot_config['properties']['repo_path'], self.buildbot_config['properties']['revision'], c["tooltool_manifest_path"]) IIUC, this will barf without explanation if we use setup_avds outside of automation. We often wrap buildbot_config refs with a 'if self.buildbot_config and 'properties' in self.buildbot_config'. I'd like to remove all references to buildbot_config soon but if we can't get repo_path or revision elsewhere, a condition and self.fatal() on an else, might be good for devs if this ever ends up outside automation and this is a loaner who can avail of tooltool

Comment on attachment 8501209 [details] [diff] [review] mozharness_v5 in-tree manifests have been added to beta, release and esr31 as well: https://hg.mozilla.org/releases/mozilla-beta/rev/0b7106ef79d2 https://hg.mozilla.org/releases/mozilla-release/rev/3f092c058c62 https://hg.mozilla.org/releases/mozilla-esr31/rev/bce0ad3815fc mozharness change landed in: https://hg.mozilla.org/build/mozharness/rev/edf6b193e300

Errata Corrige: mozharness change landed in https://hg.mozilla.org/build/mozharness/rev/fb6218e85361

In production: https://hg.mozilla.org/build/mozharness/rev/edf6b193e300

In production: https://hg.mozilla.org/build/mozharness/rev/fb6218e85361

I spot-checked the setup-avds mozharness step on tbpl and it seems to work as expected across branches (download with tooltool + correct usage of cache).

(In reply to Simone Bruno [:simone] from comment #28) > https://hg.mozilla.org/releases/mozilla-esr31/rev/bce0ad3815fc Android tests are failing on this push: https://treeherder.mozilla.org/ui/logviewer.html#?job_id=8532&repo=mozilla-esr31

"repo_path" property in buildbot_configs/mozilla-tests/mobile_config.py does not overwrite the default value set in mozilla-tests/config_common.py, as it probably should. I am going to patch this.

Comment on attachment 8508605 [details] [diff] [review] buildbot-configs: fix repo_path property for mozilla-esr31 Checked in: https://hg.mozilla.org/build/buildbot-configs/rev/26eb7aab48d4

In production: https://hg.mozilla.org/build/buildbot-configs/rev/26eb7aab48d4

The correct repo_path property seems now to be used in esr31 as well, and tests are able to retrieve releng.manifest's and successfully complete the setup_avds step (e.g., see https://tbpl.mozilla.org/php/getParsedLog.php?id=50767607&tree=Mozilla-Esr31&full=1)