workflow-templates/knative-go-build.yaml does also testing

[rhuss]

This workflow action actually does a go test which IMO has some overlap with knative-go-test.yaml.

I wonder whether this file is required at all and whether the testing in knative-go-test.yaml is not good enough ?

[markusthoemmes]

Note that it calls an empty -run parameter. It's used to make sure tests are building fine too, which go build doesn't do. Tests shouldn't actually be executed though.

[rhuss]

Can't this still be combined with the test action as the code is build there, too ? This would save resources, and if build fails, both, the build and test action would always fail anyway.

I'd suggest a single knative-go-build-test.yaml also because build doesn't really take long (i guess firing up / pulling the containers for the action can take longer)

[markusthoemmes]

I agree with the sentiment and I'm not against collapsing them.

I will say though that just running go test mixes the compiler output with the test output and can be somewhat annoying to find the actual root cause.

That being said, I dunno how often this is actually an issue as-in: How many people actually commit code that doesn't even compile and then causes them to have to debug that in CI.

[rhuss]

That being said, I dunno how often this is actually an issue as-in: How many people actually commit code that doesn't even compile and then causes them to have to debug that in CI.

+1 And if so, I guess they deserve to have to look a bit closer in the test output. But I wouldn't mind to add a prior build step for the test step to make it more evident when things are failing.

Said that, I wonder whether we shouldn't be even more flexible here and allow to plugin a build script (e.g. call that if this exists). The client uses an own "build.sh" for doing all that stuff (and a bit more), so calling this instead of a vanilla go build or go test would be the preferred way for doing that part for the Knative client.

One could add a check, that if hack/build.sh exists this script is called. Otherwise the usual go command combo is started. wdyt ?