Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: import 'sign-addon' correctly #94

Merged
merged 1 commit into from
Mar 18, 2020
Merged

fix: import 'sign-addon' correctly #94

merged 1 commit into from
Mar 18, 2020

Conversation

iorate
Copy link
Contributor

@iorate iorate commented Mar 18, 2020

Description

Checklist

  • This PR has updated documentation
  • This PR has sufficient testing

DevQA

Comments

I unsuccessfully tried to release my add-on using v0.2.4 due to the following errors:

[10:43:23 AM] [semantic-release] › ℹ  Start step "publish" of plugin "semantic-release-firefox-add-on"
Building web extension from dist/firefox/production
[10:43:23 AM] [semantic-release] › ✖  Failed step "publish" of plugin "semantic-release-firefox-add-on"
[10:43:23 AM] [semantic-release] › ✖  An error occurred while running semantic-release: TypeError: defaultAddonSigner is not a function
    at signAddon (/home/runner/work/uBlacklist/uBlacklist/node_modules/semantic-release-firefox-add-on/src/publish.js:34:30)
    at /home/runner/work/uBlacklist/uBlacklist/node_modules/web-ext/dist/web-ext.js:1:66145
    at publish (/home/runner/work/uBlacklist/uBlacklist/node_modules/semantic-release-firefox-add-on/src/publish.js:46:33)
    at validator (/home/runner/work/uBlacklist/uBlacklist/node_modules/semantic-release/lib/plugins/normalize.js:34:24)
    at /home/runner/work/uBlacklist/uBlacklist/node_modules/semantic-release/lib/plugins/pipeline.js:37:34
    at async Promise.all (index 0)
    at next (/home/runner/work/uBlacklist/uBlacklist/node_modules/p-reduce/index.js:16:18) {
  pluginName: 'semantic-release-firefox-add-on'
}
TypeError: defaultAddonSigner is not a function
    at signAddon (/home/runner/work/uBlacklist/uBlacklist/node_modules/semantic-release-firefox-add-on/src/publish.js:34:30)
    at /home/runner/work/uBlacklist/uBlacklist/node_modules/web-ext/dist/web-ext.js:1:66145
    at publish (/home/runner/work/uBlacklist/uBlacklist/node_modules/semantic-release-firefox-add-on/src/publish.js:46:33)
    at validator (/home/runner/work/uBlacklist/uBlacklist/node_modules/semantic-release/lib/plugins/normalize.js:34:24)
    at /home/runner/work/uBlacklist/uBlacklist/node_modules/semantic-release/lib/plugins/pipeline.js:37:34
    at async Promise.all (index 0)
    at next (/home/runner/work/uBlacklist/uBlacklist/node_modules/p-reduce/index.js:16:18) {
  pluginName: 'semantic-release-firefox-add-on'
}
##[error]Process completed with exit code 1.

sign-addon should be imported as const defaultAddonSigner = require('sign-addon').default.

Copy link
Contributor

@iamogbz iamogbz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

  • Do you have an idea of a good way to test for this? Since the e2e didn't catch it because semantic-release dryRun does not mock the publish step.

@iamogbz iamogbz merged commit 6033290 into tophat:master Mar 18, 2020
@iamogbz
Copy link
Contributor

iamogbz commented Mar 18, 2020

🎉 This PR is included in version 0.2.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@iorate
Copy link
Contributor Author

iorate commented Mar 19, 2020

@iamogbz Thank you!

I'm not familiar with testing and CI, but how about moving the e2e to another git repository and making it publish actually?

  1. Move the e2e to another repository.
  2. Set up the CI of the e2e to run semantic-release with dryRun: false and actually publish a fake add-on.
    • It should be avoided to publish a fake add-on to AMO. addons-dev.allizom.org seems available instead (ref).
  3. In the test of semantic-release-firefox-add-on, push a fake commit to the e2e repository and receive a result of the CI.

@iamogbz
Copy link
Contributor

iamogbz commented Mar 19, 2020

@iorate thanks for that link, will look into using the fake. Although moving it into another repo wouldn't quite work as we want to run the tests before merging or releasing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants