Closed Bug 1253736 Opened 4 years ago Closed 4 years ago

when running talos in --develop mode, run addons from source, not the signed ones

Categories

(Testing :: Talos, defect)

defect
Not set

Tracking

(firefox48 fixed)

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: jmaher, Unassigned)

References

Details

(Whiteboard: [talos_wishlist])

Attachments

(1 file, 1 obsolete file)

when we are developing addons, tests, or features to Talos, it would be really nice to have the ability to test things locally and even push to try without going through the signing dance.

In bug 1249733, we added signed addons and the requirement to use them.

I would like to make it so that when we run talos with --develop, we end up with:
* setPref: 'xpinstall.signatures.required': False
* for all extensions, trim off the leafname, e.g '${talos}/talos-powers/talos-powers-signed.xpi' -> '${talos}/talos-powers'

that should do the trick.  If we push to try with --develop, it should work as well to validate correctness.  We would have to sort out results on try, but that can be a followup bug!
:stanley, if you want a challenge, this would be a great hack to make!
Flags: needinfo?(stanleym2711)
Blocks: 1088251
Whiteboard: [talos_wishlist]
Fwiw, it's not just "really nice". The current situation means that every time I need to add a `dump()` to find out what's wrong with my (pre-existing) add-on, I have to:
1. make my changes in the code;
2. update the version number in install.rdf;
3. zip the files;
4. upload the xpi to AMO and fill the forms;
5. download the signed xpi from AMO;
6. put it back in the correct directory;
7. and now test.

That's 5 steps too many.
Yes it really sucks, but you're likely preaching to the choir :).

Though two points to make it a bit easier in the meantime:
1. No need to fill forms, see: https://wiki.mozilla.org/EngineeringProductivity/HowTo/SignExtensions
2. Signing isn't enforced in nightly/aurora. So I'd recommend you add the pref to disable signing while testing locally, then once you have it working and ready to land, sign it once at the end.
(In reply to Andrew Halberstadt [:ahal] from comment #3)
> Yes it really sucks, but you're likely preaching to the choir :).
> 
> Though two points to make it a bit easier in the meantime:
> 1. No need to fill forms, see:
> https://wiki.mozilla.org/EngineeringProductivity/HowTo/SignExtensions

As far as I can tell, this requires me rewriting the extension with Jetpack. I just want to add a `dump()`...

> 2. Signing isn't enforced in nightly/aurora. So I'd recommend you add the
> pref to disable signing while testing locally, then once you have it working
> and ready to land, sign it once at the end.

Yes, I have tried to patch Talos to do this. Now, for some reason, it seems to ignore my xpi entirely, which isn't a big improvement.
jpm can sign any .xpi, not just jetpack based ones
Didn't WFM. It doesn't seem to like .rdf-based add-ons, at the very least.
So while I think what Joel suggests above is an improvement, I personally feel that we should just use the source-based addons unless we're working against a build that requires signed-addons. Hacking on talos tests is already far too difficult, the last thing we need is additional hoops for developers to jump through.

We could make it part of the uplift process to beta to produce a signed addon.
:wlach, asking you for review on this.  The most controversial bit in the code is forcing these changes on branch_name=Try.  

at least this allow for local development and try to work it out.  A danger is pushing to try, getting green, then landing without newly signed addons.  I am not sure how to enforce that.
Flags: needinfo?(stanleym2711)
Comment on attachment 8728506 [details]
MozReview Request: Bug 1253736 - when running talos in --develop mode, run addons from source, not the signed ones. r=wlach

https://reviewboard.mozilla.org/r/38957/#review35631

There's one thing I might change (if I'm not missing something), but generally this looks good. Thanks!

::: testing/talos/talos/run_tests.py:114
(Diff revision 1)
> +        browser_config['extensions'] = ['/'.join(i.split('/')[:-1])

I think it would probably be better to use dirname here:

https://docs.python.org/2/library/os.path.html#os.path.dirname
http://stackoverflow.com/a/2817302
Attachment #8728506 - Flags: review?(wlachance)
(In reply to Joel Maher (:jmaher) from comment #9)
> :wlach, asking you for review on this.  The most controversial bit in the
> code is forcing these changes on branch_name=Try.  
> 
> at least this allow for local development and try to work it out.  A danger
> is pushing to try, getting green, then landing without newly signed addons. 
> I am not sure how to enforce that.

I don't know if there's an easy way around that. I think we just need to provide good developer documentation and hope for the best. I'm hoping this at least fixes a source of needless frustration for developers -- I think it's more common to want to modify an extension slightly for debugging purposes than to actually want to change it.
Comment on attachment 8728506 [details]
MozReview Request: Bug 1253736 - when running talos in --develop mode, run addons from source, not the signed ones. r=wlach

https://reviewboard.mozilla.org/r/38957/#review35673
Attachment #8728506 - Flags: review+
Comment on attachment 8728506 [details]
MozReview Request: Bug 1253736 - when running talos in --develop mode, run addons from source, not the signed ones. r=wlach

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/38957/diff/1-2/
Attachment #8728506 - Attachment description: MozReview Request: Bug 1253736 - when running talos in --develop mode, run addons from source, not the signed ones. r?wlach → MozReview Request: Bug 1253736 - when running talos in --develop mode, run addons from source, not the signed ones. r=wlach
Attachment #8728606 - Attachment is obsolete: true
Attachment #8728606 - Flags: review?(wlachance)
https://hg.mozilla.org/mozilla-central/rev/ab7bcfd26c6e
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.