-
Notifications
You must be signed in to change notification settings - Fork 567
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
Improve smoke tests dev experience #1767
Conversation
b80363e
to
ca471d5
Compare
exit 1 | ||
fi | ||
echo "Installing shellspec with brew" | ||
brew install shellspec |
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 is mac specific, do we plan on making linux/windows scripts later 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.
I've looked into that, but I gave up
███████╗██╔████╔██║██║ ██║█████╔╝ █████╗ ██║ █████╗ ███████╗ ██║ ███████╗ | ||
╚════██║██║╚██╔╝██║██║ ██║██╔═██╗ ██╔══╝ ██║ ██╔══╝ ╚════██║ ██║ ╚════██║ | ||
███████║██║ ╚═╝ ██║╚██████╔╝██║ ██╗███████╗ ██║ ███████╗███████║ ██║ ███████║ | ||
╚══════╝╚═╝ ╚═╝ ╚═════╝ ╚═╝ ╚═╝╚══════╝ ╚═╝ ╚══════╝╚══════╝ ╚═╝ ╚══════╝ |
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.
fancy
#!/usr/bin/env bash | ||
set -e | ||
|
||
echo "Attempting to run Smoke Tests locally. See file 'test/smoke/README.md' for details. This will drop your local 'snyk config'!" |
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.
Could you make a back up of the local config file (if it exists) before proceeding?
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.
It gets complicated in guessing the config location - would be a bit easier if snyk config
would have a command to let you know which config it is using
@@ -2,11 +2,10 @@ | |||
|
|||
echo "install snyk with binary" | |||
snyk_cli_dl=$(curl -H "Authorization: token $GITHUB_TOKEN" https://api.github.com/repos/snyk/snyk/releases/latest | jq --raw-output '(.assets[])? | select(.name == "snyk-alpine") | .browser_download_url') | |||
echo "snyk_cli_dl: ${snyk_cli_dl}" | |||
curl -Lo ./snyk-cli $snyk_cli_dl | |||
echo "snyk_cli_dl: $snyk_cli_dl" |
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 would have thought you'd want the {
/ }
.
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.
You don't need (or want?) the parameter expansion {}
if you are just printing it afaik
ca471d5
to
ed89c16
Compare
ed89c16
to
9002fcc
Compare
Merging, as it'll allow me to open another PR that will speed up the builds and allow better testing for monorepo cases such as #1707 |
Updates the ad hoc experience of running smoke tests locally
Checkout the readme changes: https://github.com/snyk/snyk/blob/smoke/improve-shellspec/test/smoke/README.md