-
Notifications
You must be signed in to change notification settings - Fork 596
add installer tests #2045
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
base: master
Are you sure you want to change the base?
add installer tests #2045
Conversation
alixander
left a comment
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.
Sorry about the delay in getting to this. I think it's a good addition. Probably in daily though. If you'd like to make the changes, including signing your commit, please update this PR. Otherwise I can make them in another.
07dc38f to
50937be
Compare
50937be to
02d4b45
Compare
that should be done now. now there's just the remaining issue of the fact the install script fails on mac-os and windows. i guess the instructions are somewhat linux flavoured |
| name: Installer Test | ||
|
|
||
| on: | ||
| push: |
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's still on push rather than as a daily job
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.
What's the rationale for not wanting this on push?
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.
slows down ci for rare changes. i suppose if you want to add a gate that says only run this if the install script changed, it can be on push, but even then it'd have to be grouped under "ci" workflow to avoid the extra noise
the installer currently fails on windows. This PR adds a job which tests the installer against different targets.
This doesn't fix the breakage, but should catch it next time.