Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add test module #55

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add test module #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions modules/test/00_mod.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Copyright 2023 The cert-manager Authors.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

shared_test_targets ?=
go_test_targets ?=
junitfile ?=
27 changes: 27 additions & 0 deletions modules/test/02_mod.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Copyright 2023 The cert-manager Authors.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

## Run golang tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "golang" mean in this context?
Generally we have unit and e2e tests using both ginkgo and gotestsum.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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))
Copy link
Member

@inteon inteon Jan 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The challenge is that unit tests sometimes use gotestsum and e2e tests use ginkgo or another combination of both.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@ThatsMrTalbot ThatsMrTalbot Jan 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have given me an idea for an improvement though 🙂


shared_test_targets += test-go

.PHONY: test
## Run all test targets
## @category [shared] Test
test: $(shared_test_targets)