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

Add helm-unittest #284

Merged
merged 12 commits into from
Feb 18, 2022
Merged

Add helm-unittest #284

merged 12 commits into from
Feb 18, 2022

Conversation

cognifloyd
Copy link
Member

Per #28 and #283 (review) unittests will use helm-unittest.

This adds a basic GHA workflow and a single dummy test. The dummy test is commented to document the various options in the test file.

@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label Feb 17, 2022
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for composing a quick example, looks really sleek! 👍

Do we want to split the directory structure here, considering we now have unit vs integration tests?
https://github.com/StackStorm/stackstorm-ha/tree/master/tests
Adding a new Readme.md for Unit tests would be helpful as well.

I'd welcome the changelog as that'll help other developers seeing unit test framework addition as another stability message.

@@ -18,4 +47,4 @@ To show the test results:
kubectl logs <release-name>-st2tests
```

See https://helm.sh/docs/developing_charts/#chart-tests with more information about Helm chart tests.
See https://helm.sh/docs/topics/chart-tests/ with more information about Helm chart tests.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

old link was to helm v2 docs. This is the same doc but for helm v3.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting that!

tests/README.md Outdated Show resolved Hide resolved
@cognifloyd
Copy link
Member Author

I added tests for custom annotations and removed the dummy unit test.

Comment on lines 67 to 69
- it: Deployments+Pods accept custom annotations
template: deployments.yaml
set:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing all the deployment annotations at once is an optimization. I had one test per deployment, but that meant one helm template run per deployment, which made things very very slow (no I didn't time the multi-minute slow runs, but I got the test run down to about a minute on my machine).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem with running helm unittest --helm3 -f tests/unit/*_test.yaml to structure the tests in unit/ and integration/ dirs.

Looks like the others were successful in this already:
https://github.com/jenkinsci/helm-charts/blob/26affd02465f7ef444967b0528142a6cd6b9642a/ct.yaml#L8

Let's please do something similar.

I understand it comes with a price of adding another -f tests/unit/*_test.yaml CLI param, but the other side is that the structure would be more obvious and predictable for everyone.

tests/README.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
cognifloyd and others added 3 commits February 18, 2022 08:44
Co-authored-by: Eugen Cusmaunsa <stackstorm@armab.io>
Drop the warnings/complaints about how awkward it is to use helm-unittest.

Co-authored-by: Eugen Cusmaunsa <stackstorm@armab.io>
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big thanks for all the changes! 👍

So I understand the groundwork here will help you to ship the #218.
+100 on the approach.

tests/README.md Outdated Show resolved Hide resolved
Co-authored-by: Eugen Cusmaunsa <stackstorm@armab.io>
@cognifloyd cognifloyd merged commit e10d33d into master Feb 18, 2022
@cognifloyd cognifloyd deleted the helm-unittest branch February 18, 2022 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/CD Helm size/L PR that changes 100-499 lines. Requires some effort to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants