Skip to content
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

Merged
merged 29 commits into from
Feb 4, 2021
Merged

Action tests #19

merged 29 commits into from
Feb 4, 2021

Conversation

cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Jan 28, 2021

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 on This demonstrates how StackStorm-Exchange/ci#101 will behave by running the setup_testing_env.sh script immediately following dependencies.

@cognifloyd
Copy link
Member Author

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
tests/test_action_create_token.py Outdated Show resolved Hide resolved
Copy link
Contributor

@blag blag left a 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!

tests/vault_action_tests_base.py Outdated Show resolved Hide resolved
tests/vault_action_tests_base.py Outdated Show resolved Hide resolved
tests/vault_action_tests_base.py Outdated Show resolved Hide resolved
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.
Copy link
Member Author

@cognifloyd cognifloyd left a 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.

tests/vault_action_tests_base.py Outdated Show resolved Hide resolved
@cognifloyd cognifloyd requested a review from blag February 3, 2021 23:23
Copy link
Contributor

@blag blag left a 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.

tests/vault_action_tests_base.py Outdated Show resolved Hide resolved
Co-authored-by: blag <blag@users.noreply.github.com>
Copy link
Contributor

@blag blag left a 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!

@blag blag merged commit aac0c37 into StackStorm-Exchange:master Feb 4, 2021
@blag blag mentioned this pull request Feb 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants