Conversation
Signed-off-by: Adam Talbot <adam.talbot@venafi.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| ## Run golang tests |
There was a problem hiding this comment.
What does "golang" mean in this context?
Generally we have unit and e2e tests using both ginkgo and gotestsum.
There was a problem hiding this comment.
I was just thinking unit tests since e2e tests usually require extra setup so should setup their own target. I can reword the target to test-go-unit or something?
| ## @category [shared] Test | ||
| .PHONY: test-go | ||
| test-go: | $(NEEDS_GOTESTSUM) | ||
| $(if $(go_test_targets),$(GOTESTSUM) $(addprefix --junitfile=, $(junitfile)) -- $(go_test_targets)) |
There was a problem hiding this comment.
Is this general enough to model tests like https://github.com/cert-manager/approver-policy/blob/main/make/test-smoke.mk#L58-L63 here? & is that what we are aiming for?
There was a problem hiding this comment.
Depends if we care much about the runner, I only used gotestsum because I saw cert-manager repo using it and it already existed as a tool. Happy to use ginkgo instead if thats easier.
Of could use a flag to specify what runner you want then have a ifdef else statement here
There was a problem hiding this comment.
The challenge is that unit tests sometimes use gotestsum and e2e tests use ginkgo or another combination of both.
There was a problem hiding this comment.
You could not define any go_test_targets and just add your own target to shared_test_targets. Then you could use whatever runner you like.
There was a problem hiding this comment.
You have given me an idea for an improvement though 🙂
Adds a simple module to add targets to a shared "make test" target. Also includes a built in target for testing golang packages