docker-mailserver-helm icon indicating copy to clipboard operation
docker-mailserver-helm copied to clipboard

restore helm unittests

Open MichaelSp opened this issue 1 year ago • 9 comments

For some reason the unit-tests are quite outdated and are not working anymore. This PR restores the unittests and provides a GitHub Actions workflow to run it on PRs

MichaelSp avatar May 26 '24 12:05 MichaelSp

Cool - happy to have a PR that fixes unit tests, but this PR also includes other additional changes. Could you separate those out?

Also, what does the __snapshot__ directory do and why the snap file extension after yaml?\

cfis avatar May 26 '24 21:05 cfis

Additional changes? You mean fixes in deployment.yaml and typos in values.yaml?

the snapshots will be the generated version that defines the current truth. If you plan to change anything that would change the output, the test will fail because it might be a regression. You can now review the changes in the output of the helm untittest run and decide if you want it or not. If you want to continue with the changes you can run helm unittest -u to update the snapshots. The files will also be commited in to the repo. The yaml.snap files are auto-generated by the tests. All of that is documented in https://github.com/helm-unittest/helm-unittest

MichaelSp avatar May 28 '24 15:05 MichaelSp

Yes, I would like an PR that only contains the changes needed for updating the unit tests. Other changes we can talk about in separate PRs.

Thanks for the explanation on snapshots, that sounds good.

cfis avatar May 28 '24 20:05 cfis

Just following up - would like to get this merged.

cfis avatar Jun 25 '24 03:06 cfis

Ping?

cfis avatar Jul 09 '24 04:07 cfis

Sorry this is a bit off-topic, but just curious @MichaelSp , is SAP using DMS? :)

polarathene avatar Jul 09 '24 05:07 polarathene

Sorry for the delay here. I couldn't find the time yet to split the PR and timewise it doesn't seem to get any better... (maybe you can accept it in one PR?)

@polarathene AFAIK they are not using DMS 🤣

MichaelSp avatar Jul 10 '24 05:07 MichaelSp

@MichaelSp understood - if I find some time I'll see what I can do.

cfis avatar Jul 10 '24 21:07 cfis

Finally I was able to polish this PR a bit. One thing I noticed:

readonly is not a valid param according to the spec https://kubernetes.io/docs/concepts/storage/volumes/. The correct param would be readOnly. I assume this field is ignored, but since I assume the state should not be readOnly, I took the liberty to remove that.

MichaelSp avatar Sep 12 '24 10:09 MichaelSp

@cfis pong :)

MichaelSp avatar Sep 29 '24 09:09 MichaelSp

Oh sorry, totally missed your last update. Thanks for updating!

cfis avatar Sep 29 '24 23:09 cfis