stakater / application

Generic helm chart for all kind of applications
http://stakater.com
MIT License
213 stars 88 forks source link

Add some basic unit tests and update CI workflow #218

Closed d3adb5 closed 1 year ago

d3adb5 commented 1 year ago

Hi there! Noticed you guys were using the GitHub action I wrote for running Helm template unit tests, and decided to come check out your repository. I noticed the test suite added by @TehreemNisa in #214 wasn't actually running due to the filename used, and decided to fix that.

I realize this is a somewhat large first PR, but I figured it was worth adding some unit tests and refactoring some of the existing templates to improve readability and maintainability.

The CI workflow was updated to use v2 of my unit test action. This version points to the now official repository, taken over by the plugin maintainer.

Here is a breakdown of the changes, as a list of commits:

If this pull request is accepted and merged, I'd be happy to implement more unit tests and improve readability for the chart overall.

stakater-user commented 1 year ago

@d3adb5 Yikes! You better fix it before anyone else finds out! Build has Failed!

d3adb5 commented 1 year ago

I imagine the build failing has something to do with me not having access to repository / organization secrets. As for the unit test job, while it claims to be passing, the logs suggest my changes to the workflow weren't applied for this run, since it still uses v1 of d3adb5/helm-unittest-action, and shows no test suite being executed.

rasheedamir commented 1 year ago

thank you @d3adb5 for such a thorough PR! we are fixing the pipelines runtime and then we would really appreciate more PRs which can improve the readability of the chart and have more unit tests; we are also thinking to add integration tests

d3adb5 commented 1 year ago

thank you @d3adb5 for such a thorough PR! we are fixing the pipelines runtime and then we would really appreciate more PRs which can improve the readability of the chart and have more unit tests; we are also thinking to add integration tests

Sure thing! I'd be happy to do some refactoring and security touch ups soon! Do you guys know what's going on with the CI builds, @rasheedamir? Maybe I could help with that effort.

rasheedamir commented 1 year ago

@d3adb5 the pipelines are fixed now! can you please update this PR or create new one? now we are using self hosted github runners on full blown openshift cluster to ensure stuff works

d3adb5 commented 1 year ago

Seems like plenty changed in the repository. Well, I rebased my changes and hopefully everything's working fine. A careful review would be much appreciated! Thanks, @rasheedamir.

github-actions[bot] commented 1 year ago

@d3adb5 validation successful`

rasheedamir commented 1 year ago

@slauger @aslafy-z can you please review it

d3adb5 commented 1 year ago

@aslafy-z Conflicts resolved! Turns out #221 did what one of my previous commits was doing. I changed my unit tests and commit messages accordingly.

d3adb5 commented 1 year ago

Is it okay to merge this, @rasheedamir?