-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
ci: replace travis with actions, setup renovate #1699
Conversation
114ec95
to
7083d0e
Compare
it looks like the fact that the tests are ran on GitHub actions somehow interferes with the tests. This is the last step to setup renovate across @semantic-release, would be great to get this fixed. I can have another look next week, but any help is welcome |
That's correct. Adding a couple of env vars takes the error count from 8 to 3: The error I hit involves an unexpected URL -- happy to return the adventure to you :-). |
7083d0e
to
1f71b2b
Compare
53a6e8b
to
3c3b20c
Compare
I think I got it 🤞🏼 mocking the environment variables by setting values to |
That results in the value `"null"` instead of undefined
that didn't work, but I'm pretty sure the problem is caused because |
…the repository URL as is if none of the given git credentials are valid" which should never have passed before but well here we are
I think I needed to set:
I also looked into setting up multiple repo servers, because it felt like tests might be colliding... |
I think we don't want to set any of the I feel like there is a racing condition: https://github.com/semantic-release/semantic-release/runs/1737560659#step:7:372 It fails because the request URL it expects for the |
Yeah, that's where I think you need to use distinct dockers for each test, it's the same fail I'm hitting. |
I don't get the same error locally.
I don't know how I would even do that right now :D I still hope to make it work, I've changed the tests to run sequentially, let's see ... |
This comment has been minimized.
This comment has been minimized.
Today I learned: turns out that |
I think I got it now, thank you @jsoref, you helped a lot :) |
@@ -122,13 +122,19 @@ | |||
"lint": "xo", | |||
"pretest": "npm run lint", | |||
"semantic-release": "./bin/semantic-release.js", | |||
"test": "nyc ava -v" | |||
"test": "nyc ava -v", | |||
"test:ci": "nyc ava -v" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it worth having this as a separate script when it already matches the existing test
script? i know we needed it, at least temporarily, in some cases where they didnt match, but seems like that could be avoided here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added it because of pretest
. On CI, we run the linting in the separate job, that only runs after the tests passed in all supported node versions
TRAVIS_BRANCH: 'master', | ||
TRAVIS_PULL_REQUEST: 'false', | ||
GITHUB_API_URL: mockServer.url, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main problem why the CI tests started to fail after migrating from Travis to GitHub Actions was that the environment variables set by the GitHub Actions environment sneaked into process.env
. I always thought that setting the env
option for exec would set all environment variables, but instead it only extends process.env
. Setting extendEnv
resolved the problem
🎉 This PR is included in version 17.3.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
push: | ||
branches: | ||
- master | ||
- renovate/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I claim this is wrong ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or perhaps, if it's intended, I'd suggest '**renovate**'
and then document that people can test renovate stuff by using a branch vaguely named renovate -- there's no particular explanation anywhere in this PR as to why renovate
is special as part of a branch name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using https://github.com/renovatebot to keep dependencies up to date and it creates branches that are named to match this pattern. This ensures that these branches trigger this workflow, even in the cases we have configured renovate to not open pull requests (although we still have a few kinks to work out around those cases). This pattern isn't intended to enable any special behavior for outside contributors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that isn't particularly obvious (there aren't enough hints anywhere). Might I suggest a # renovate/* branches are generated by @renovate-bot
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great idea! Let's do that! Pull request welcome ;)
Co-authored-by: Josh Soref <jsoref@users.noreply.github.com>
fixes #1692