-
Notifications
You must be signed in to change notification settings - Fork 63
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
test: Adding scheduled actions to test latest CLI release #1138
Conversation
run: npm install | ||
- name: Test Latest CLI Release | ||
run: | | ||
npm run test-cli --cliNPMVersion="5.0.2" |
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.
Purposefully using a bad CLI release on the first pass so we can watch this fail
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.
Don't forget to update this to latest
before merging!
@@ -46,7 +44,7 @@ jobs: | |||
status: ${{ job.status }} | |||
notify_when: 'failure' | |||
notification_title: 'CLI build failure' | |||
message_format: '{emoji} *Build* {status_message} in <{repo_url}|{repo}> on <{commit_url}|{commit_sha}>"' |
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.
This was a typo by me from another PR
@@ -0,0 +1,38 @@ | |||
on: |
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.
There are two ways this will run:
- Every time a new tag is published
- Every 5 hours
The sleep at the beginning of the action is to give the newly-published package some time to become available on npm. Otherwise latest
may not target the most recent release.
acceptance-tests/run-tests
Outdated
description: 'CLI path', | ||
description: 'The path to the CLI', | ||
}) | ||
.option('cliNPMVersion', { |
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.
Open to suggestions for a better arg 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.
I feel like just calling it cliVersion
might be clearer? cliNPMVersion
kind of makes it sound like its the version of npm
you're going to use. But don't feel super strongly
|
||
args = [processPath].concat(args); | ||
if (cliNPMVersion) { | ||
processCommand = 'npx'; |
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.
This will use npx to download and execute cli commands for each test. It ends up looking something like this:
npx --yes --package=@hubspot/cli@latest hs init
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.
This looks great! Pleasantly surprised that it was this simple to get all this working. Just a few minor suggestions
acceptance-tests/run-tests
Outdated
description: 'CLI path', | ||
description: 'The path to the CLI', | ||
}) | ||
.option('cliNPMVersion', { |
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 feel like just calling it cliVersion
might be clearer? cliNPMVersion
kind of makes it sound like its the version of npm
you're going to use. But don't feel super strongly
acceptance-tests/run-tests
Outdated
.option('cliNPMVersion', { | ||
alias: 'npm', | ||
type: "string", | ||
description: "The npm version of the CLI to download and run" |
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.
Might make sense to say that specifying a version will cause the tests to run with npx
, rather than "download and run" (even though npx technically does download"
run: npm install | ||
- name: Test Latest CLI Release | ||
run: | | ||
npm run test-cli --cliNPMVersion="5.0.2" |
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.
Don't forget to update this to latest
before merging!
@camden11 I'm actually going to merge this with the |
Description and Context
Adding a cron github action that runs once per hour to pull the latest CLI release and test out a command. The idea here is to catch any major issues in releases as soon as possible. I'm also configuring a slack notifications bot to post in our team's channel whenever the github actions fail.
Screenshots
TODO
Who to Notify