The create_test_files.sh script in test_signed_apps/gentestfiles creates some test files that are no longer used. They are (at least) : corrupt_app_1.zip origin_app_1.zip valid_app_2.zip (all removed in bug 1261019) custom_origin.zip (removed in bug 1238079 It's unclear that these are still used: unsigned_app_origin.zip unsigned_app_origin_toolkit_webapps.zip

There's some more, looks like: privileged-app-test-1.0.zip test-privileged-app-test-1.0.zip (uses removed in bug 1332636) Also, it looks like unsigned_app_origin and unsigned_app_origin_toolkit_webapps were only used to create origin_app_1.zip and custom_origin.zip, respectively, so those can be removed as well.

Actually, sorry, scratch this for now. I think we're going to need to remove this entirely and replace it with something more like pycert, so we have more flexibility in creating testcases.

Comment on attachment 8917576 [details] bug 1407081 - rework signed app tests for flexibility with upcoming hash algorithm changes https://reviewboard.mozilla.org/r/188538/#review194620 OK. There's an awful lot in here, but it seems to work OK and I don't see anything wrong. ::: security/manager/ssl/tests/unit/test_signed_apps/app/README:1 (Diff revision 1) > +This is the readme for the test extension. And so it is. ::: security/manager/ssl/tests/unit/test_signed_apps/app/manifest.json:3 (Diff revision 1) > +{ > + "manifest_version": 2, > + "name": "Text Extension", Not "Test Extension"?

Comment on attachment 8917576 [details] bug 1407081 - rework signed app tests for flexibility with upcoming hash algorithm changes https://reviewboard.mozilla.org/r/188538/#review195954 Sorry for the delay. This looks reasonable, but the most simple way of running create_test_files.sh doesn't seem to work, so I'm going to withhold r+ until I can actually run and test out the code (maybe I'm just doing something wrong). ::: security/manager/ssl/tests/unit/pycms.py:43 (Diff revision 1) > +class UnknownDirectiveError(Error): > + """Helper exception type to handle unknown specification > + directives.""" > + > + def __init__(self, directive): > + super(Error, self).__init__() I believe the first call to `super()` should instead be `UnknownDirectiveError`. ::: security/manager/ssl/tests/unit/pycms.py:45 (Diff revision 1) > + directives.""" > + > + def __init__(self, directive): > + super(Error, self).__init__() > + self.directive = directive > + self.category = 'input' This appears to be unused. ::: security/manager/ssl/tests/unit/test_signed_apps/create_test_files.sh:3 (Diff revision 1) > +#!/bin/bash > + > +./sign_app.py -d app/ -o signed_app.zip I tried to run this, and I ran into the following: $ ./create_test_files.sh Traceback (most recent call last): File "./sign_app.py", line 17, in <module> import pycms ImportError: No module named pycms

Comment on attachment 8917576 [details] bug 1407081 - rework signed app tests for flexibility with upcoming hash algorithm changes https://reviewboard.mozilla.org/r/188538/#review194620 Thanks! > Not "Test Extension"? Heh - fixed.

Comment on attachment 8917576 [details] bug 1407081 - rework signed app tests for flexibility with upcoming hash algorithm changes https://reviewboard.mozilla.org/r/188538/#review195954 Thanks! I was using PYTHONPATH to make create_test_files.sh work, but then I figured out how to integrate the file generation into the build system (and then comment it out because of bug 1256495 ¯\_(ツ)_/¯ ). Let me know what you think. > I believe the first call to `super()` should instead be `UnknownDirectiveError`. Ah - fixed. > This appears to be unused. Whoops - copy/paste. Fixed. > I tried to run this, and I ran into the following: > > $ ./create_test_files.sh > Traceback (most recent call last): > File "./sign_app.py", line 17, in <module> > import pycms > ImportError: No module named pycms Yeah, I probably should have put this all in a moz.build in the first place anyway. See security/manager/ssl/tests/unit/test_signed_apps/moz.build in the upcoming patch.

Comment on attachment 8917576 [details] bug 1407081 - rework signed app tests for flexibility with upcoming hash algorithm changes https://reviewboard.mozilla.org/r/188538/#review196452 Great! Just the one comment about making sure the moz.build mechanism works properly if we ever choose to uncomment it. ::: security/manager/ssl/tests/unit/test_signed_apps/moz.build:18 (Diff revision 2) > + props = GENERATED_FILES[name] > + props.script = '/security/manager/ssl/tests/unit/sign_app.py' > + props.inputs = ['app/'] > + if flags: > + props.flags = flags > + AFAICT, the current template definition does produce the files as expected, but we're going to need to doing something like https://hg.mozilla.org/mozilla-central/file/20d57b9c4183973af4af5e078dff2aec0b74f928/build/test_templates.mozbuild#l16 as well for the files to actually be moved to a location that xpcshell tests can access.

Comment on attachment 8917576 [details] bug 1407081 - rework signed app tests for flexibility with upcoming hash algorithm changes https://reviewboard.mozilla.org/r/188538/#review196452 > AFAICT, the current template definition does produce the files as expected, but we're going to need to doing something like https://hg.mozilla.org/mozilla-central/file/20d57b9c4183973af4af5e078dff2aec0b74f928/build/test_templates.mozbuild#l16 as well for the files to actually be moved to a location that xpcshell tests can access. Ah - good catch. Thanks!

Thanks for the reviews! https://treeherder.mozilla.org/#/jobs?repo=try&revision=6da0fbc42049

Pushed by dkeeler@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/544b2dfa1b06 rework signed app tests for flexibility with upcoming hash algorithm changes r=Cykesiopka,jcj