semantic-release / travis-deploy-once

🚫Test multiple node versions on Travis. Deploy once. If all of them pass.
MIT License
33 stars 7 forks source link

Suddenly stopped working for my pull requests #66

Closed kentcdodds closed 6 years ago

kentcdodds commented 6 years ago

I probably need to investigate further, but does anyone know off the top of their head why something like this would stop working on a pull request?

console.log('installing and running travis-deploy-once')
const deployOnceResults = spawn.sync('npx', ['travis-deploy-once@5'])
if (deployOnceResults.status === 0) {
  console.log('do stuff!')
} else {
  process.exit(deployOnceResults.status)
}

This is slightly modified from kcd-scripts:

https://github.com/kentcdodds/kcd-scripts/blob/ae04401a6cc0e6e02d2050e0af164098d7f3608f/src/scripts/travis-after-success.js#L10-L16

All my projects use this script as the after success hook and they've stopped running in pull requests (master branch works fine).

Here's an example pull request with this problem:

The resulting issue is that codecov is never run so that status check is never reported on the GitHub PR.

Any ideas? Or do I need to dig deeper?

pvdlg commented 6 years ago

I don't know why it would not work in this specific case. The only thing I can think of is that the GitHub token set in the Travis environment variable is not valid anymore. The token is used to authenticate with the Travis API.

For information we provide a JS API. See https://github.com/semantic-release/travis-deploy-once#api. Actually travis-deploy-once used to be API only and the CLI was added recently.

As you call travis-deploy-once from JS code that would be a lot easier for you to use the API rather than calling the CLI with cross-spawn. In addition the API would throw an exception is something goes wrong (like if the token is expired, or if the API is innaccessible) so you could log the exception to get more informations.

kentcdodds commented 6 years ago

Thank you @pvdlg! I may want to refactor to the node API. The reason I didn't use that is because I wanted to use npx so the project doesn't have to have it installed for that to work.

It looks like it started working again in this pr: https://github.com/kentcdodds/react-testing-library/pull/84 so we'll see if any changes are necessary. Thanks!

pvdlg commented 6 years ago

The reason I didn't use that is because I wanted to use npx so the project doesn't have to have it installed for that to work.

I'm not sure I'm understanding your point. By using cross-spawn to call npx travis-deploy-once@4, kcd-scripts has a dependency to travis-deploy-once. What would be the difference if you would define travis-deploy-once as a dependency in the package.json of kcd-scripts and call it via API? In both case the project using kcd-scripts doesn't have to have travis-deploy-once installed.

kentcdodds commented 6 years ago

The difference is that I only need travis-deploy-once on CI, not locally. If I put it in the package.json then it would need to be installed by anyone using kcd-scripts. By using npx it's only used when someone runs kcd-scripts travis-after-success :)

pvdlg commented 6 years ago

Ah ok I get it, thanks for the clarification! I never had a similar situation, but I imagine that "conditional dependencies" is not that an uncommon use case. I'd be curious to know if it's something in the npm/yarn roadmap.

pvdlg commented 6 years ago

It looks like it started working again in this pr: kentcdodds/react-testing-library#84 so we'll see if any changes are necessary. Thanks!

So it seems the problem was external to travis-deploy-once. I'm going to close the issue for now, please re-open if the problem happens again and you think it's due to travis-deploy-once.