Skip to content
This repository has been archived by the owner on Nov 27, 2019. It is now read-only.

Use sign-addon for the sign command (closes #519) #546

Merged
merged 1 commit into from
Jul 11, 2016
Merged

Use sign-addon for the sign command (closes #519) #546

merged 1 commit into from
Jul 11, 2016

Conversation

yan12125
Copy link
Contributor

@yan12125 yan12125 commented Jul 9, 2016

No description provided.

@yan12125
Copy link
Contributor Author

yan12125 commented Jul 9, 2016

Lots of Travis CI tests fail with strange error messages:

[5576] WARNING: firefox: Fatal IO error 11 (Resource temporarily unavailable) on X server :99.0.

: 'glib warning', file /builds/slave/m-beta-l64-0000000000000000000/build/src/toolkit/xre/nsSigHandlers.cpp, line 142

On my local machine, with Firefox 47.0.1 and node.js 6.3.0, all 178 tests in JPM_FIREFOX_BINARY=which firefox xvfb-run npm test pass.

@yan12125
Copy link
Contributor Author

yan12125 commented Jul 9, 2016

Re-pushed with removing the last AMOClient from lib/sign.js

@yan12125
Copy link
Contributor Author

OK apparently firefox-beta is broken.

@freaktechnik
Copy link
Contributor

I think the problem here is, that beta once again enforces signed extensions while there is an unbranded build somewhere out there. But I'm not sure as I've not seen any official announcement nor complaints from developers and haven't checked myself. @kumar303 probably knows more.

@kumar303
Copy link
Contributor

Hmm, I don't know much about the mechanics of the functional tests actually. I would assume we need latest-nightly but I guess lately is working since tests are passing :/

@kumar303
Copy link
Contributor

This is looking great! For some reason, the api configuration is not covered in the tests (I think this was previously covered by the lower level request tests maybe). Could you add a test similar to it("passes manifest info to the signer", ...) for this?

As an example, when I comment out these parameters, the tests should fail but for me they are passing:

diff --git a/lib/sign.js b/lib/sign.js
index 46a52ce..f69154c 100644
--- a/lib/sign.js
+++ b/lib/sign.js
@@ -113,9 +113,9 @@ function sign(options, config) {
       xpiPath: xpiInfo.xpiPath,
       id: xpiInfo.id,
       version: xpiInfo.version,
-      apiKey: options.apiKey,
-      apiSecret: options.apiSecret,
-      apiUrlPrefix: options.apiUrlPrefix,
+      //apiKey: options.apiKey,
+      //apiSecret: options.apiSecret,
+      //apiUrlPrefix: options.apiUrlPrefix,
       verbose: options.verbose,
       timeout: options.timeout || undefined,
     }).then(function(result) {

@kumar303
Copy link
Contributor

I also just released 0.1.1 of sign-addon, can you switch it to that while you're in there?

@yan12125
Copy link
Contributor Author

Thanks all done. sign-addon bumped to 0.1.1 and API tests added.

@kumar303
Copy link
Contributor

This test failure looks bogus. I restarted the build. It looks like very high I/O times on TravisCI, which I've seen before.

@kumar303
Copy link
Contributor

+37 −1,597

more-red-than-green-award

@kumar303
Copy link
Contributor

If this test passes (I think it will) then it will be good to go. However, I'd like to see all these commits squashed into one so that the commit history is easier to read.

@yan12125
Copy link
Contributor Author

Squash done :)

@kumar303
Copy link
Contributor

Thanks! @freaktechnik just showed me https://github.com/blog/2141-squash-your-commits for next time :)

@kumar303 kumar303 merged commit a7e0642 into mozilla-jetpack:master Jul 11, 2016
@yan12125 yan12125 deleted the sign-addon-for-sign-command branch July 11, 2016 21:09
@kumar303 kumar303 self-assigned this Sep 1, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants