-
Notifications
You must be signed in to change notification settings - Fork 12
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
Action tests #19
Action tests #19
Conversation
880df9f
to
e8a95e7
Compare
I dropped 2.7 tests due to: StackStorm/community#40 (comment) 3.6 tests are failing because StackStorm-Exchange/ci#101 hasn't been merged yet. |
Since the 2.7 job was dropped, we need to save things in the 3.6 job so that it is available during the deploy step. Also add some safety bits that are in the StackStorm-Exchange/ci and should eventually go to all packs.
In [1] the default path was changed from ${HOME}/bin to ${HOME}/.local/bin which is compatible with both CI and local development, so drop the hacky workaround. [1] hvac/hvac@56ade59
41d5e8f
to
c8760f6
Compare
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 suggest using super()
where we can. Here's a write up from Raymond Hettinger (famous Pythonista) about super()
.
TL;DR: It leads to less code churn as parent classes change.
Other than that, this looks great!
We still have to have two calls, because HvacIntegrationTestCase does not call super(). However, we can call super() for it, so do that instead of the old manual method.
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.
Fascinating. I wasn't sure how to deal with calling super() when one of the parents didn't also call super(). Still using super(SuperCallLessClass, self)
is a nicer work around. I've pushed the changes and adjusted the comments to help future me understand what's going 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.
One small mistake needs to be fixed.
Co-authored-by: blag <blag@users.noreply.github.com>
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.
Awesome, thanks for taking this on!
Add tests for all actions.
To do this, we reuse the hvac testing infra to setup consul/vault for our tests.
Also, we drop the broken python2.7 tests and update the CI config to use the latest from StackStorm-Exchange/ci.
This depends onThis demonstrates how StackStorm-Exchange/ci#101 will behave by running thesetup_testing_env.sh
script immediately followingdependencies
.