feat(test): Enable local testing with taskfile - #39
Conversation
maximiliantech
left a comment
maximiliantech
left a comment
There was a problem hiding this comment.
Nice change @jakobve 🫶 I do have some thoughts down below. What do you think?
| run: once | ||
| cmds: | ||
| - mkdir -p {{.LOCALBIN}} | ||
| - GOBIN={{.LOCALBIN}} go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest |
There was a problem hiding this comment.
I am not quite sure wether we should use go install. Based on their documentation it is recommended to install the binaries via https://golangci-lint.run/docs/welcome/install/local/#binaries. Could you please check @jakobve?
| - > | ||
| {{.ENVTEST_SETUP}} use {{.ENVTEST_K8S_VERSION}} --bin-dir {{.LOCALBIN}} | ||
|
|
||
| test:ci: |
There was a problem hiding this comment.
I love this change. Just thinking out loud here: Usually, to be really sure that we are doing the same steps (including downloaded and used versions) in CI and locally, I would like to see that in the GH action simply the task test:ci would get called so we don't need to maintain to separate locations for dependency updates. Can we accomplish that with this setup?
What do you think about it @christophrj?
|
While discussing this with @christophrj, it might be best to switch to the embed the git submodule build repository in the same way we have done this for https://github.com/openmcp-project/openmcp-testing? There we would get all of that in the same look and feel out of the box. What do you say @jakobve? |
|
@maximiliantech @christophrj, thanks for having a look at this! Why aren't you using the same approach for the service-providers? 😅 |
I thought it wasn't required for such a small go lib but it definitely makes sense for the sake of consistency. Thanks @jakobve for bringing this up. |
christophrj
left a comment
christophrj
left a comment
There was a problem hiding this comment.
Hi @jakobve sorry about the late reply. While I generally like the idea of the PR of adding our build module for consistency and reusing existing functionality, I believe there are a couple of problems when it comes to the envtest setup/execution. Please have a look at my comments and let me know if anything is unclear. I am not sure if the build module provides the required functionality in a reusable way. If not, I would suggest to just leave some of the old task definitions and make sure they are executed in the CI run.
Signed-off-by: Jakob Bergmeier <jakob.vetter@sap.com>
f71370b to
fff111b
Compare
Signed-off-by: Jakob Bergmeier <jakob.vetter@sap.com>
christophrj
left a comment
christophrj
left a comment
There was a problem hiding this comment.
@jakobve Thanks for the update. Please update the README (replace the outdated task references) and add the testdata directory as CODE_DIR, then we can merge this 👍
Signed-off-by: Jakob Bergmeier <jakob.vetter@sap.com>
christophrj
left a comment
christophrj
left a comment
There was a problem hiding this comment.
@jakobve one last thing 😆 didn't realize you changed the job name.
| jobs: | ||
| build: | ||
| runs-on: ubuntu-latest | ||
| build_validate_test: |
There was a problem hiding this comment.
We need to keep the build name, otherwise it doesn't match the repo ruleset.
Signed-off-by: Jakob Bergmeier <jakob.vetter@sap.com>
3 participants
What this PR does / why we need it:
Special notes for your reviewer:
Release note: