Conversation
|
Hello! Thank you for your contribution. Please review our contribution guidelines to understand the project's testing and code conventions. |
There was a problem hiding this comment.
Pull request overview
This pull request modernizes the test infrastructure and mock generation setup for the actions-runner-controller project. The main changes migrate from deprecated kubebuilder-tools to the official envtest setup, and update mockery from version 2.36.1 to a newer version with different mock generation patterns.
Changes:
- Migrated from kubebuilder-tools to setup-envtest for test environment binaries (etcd, kube-apiserver, kubectl)
- Updated mockery configuration from command-line flags to a centralized
.mockery.yamlconfiguration file - Regenerated all mocks with updated mockery version, changing mock naming conventions (e.g.,
MockPublisherinstead ofPublisher) - Updated test files to use new mock constructor names (e.g.,
NewMockPublisherinstead ofNewPublisher)
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.mockery.yaml |
New centralized configuration for mockery mock generation |
.gitignore |
Removed deprecated /test-assets directory reference |
Makefile |
Replaced kubebuilder-tools setup with setup-envtest utility |
.github/workflows/go.yaml |
Updated CI to use setup-envtest instead of downloading kubebuilder binaries |
.github/workflows/arc-publish.yaml |
Removed unnecessary kubebuilder installation from release workflow |
CONTRIBUTING.md |
Updated documentation to reflect new envtest setup process |
github/actions/client.go |
Simplified mockery directive from --inpackage --name=ActionsService to just mockery |
github/actions/sessionservice.go |
Simplified mockery directive from --inpackage --name=SessionService to just mockery |
github/actions/mocks_test.go |
Regenerated mocks for ActionsService and SessionService with new naming |
github/actions/mock_ActionsService.go |
Deleted old separate mock file (consolidated into mocks_test.go) |
github/actions/mock_SessionService.go |
Deleted old separate mock file (consolidated into mocks_test.go) |
cmd/ghalistener/metrics/metrics.go |
Simplified mockery directives |
cmd/ghalistener/metrics/mocks/*.go |
Regenerated mocks with new naming conventions |
cmd/ghalistener/app/app.go |
Simplified mockery directives |
cmd/ghalistener/app/mocks/*.go |
Regenerated mocks with new naming conventions |
cmd/ghalistener/app/app_test.go |
Updated to use new mock constructor names |
cmd/ghalistener/listener/listener.go |
Simplified mockery directives |
cmd/ghalistener/listener/metrics_test.go |
Updated to use new mock constructor names |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| filename: "{{.InterfaceName|snakecase}}.go" | ||
| interfaces: | ||
| Listener: | ||
| Worker: |
There was a problem hiding this comment.
The mockery configuration file is incomplete. It's missing the configuration for the cmd/ghalistener/listener package which has two interfaces (Client and Handler) that have //go:generate mockery directives. Without this configuration, running go generate may not generate the expected mocks for these interfaces, or the mocks may be generated with incorrect settings.
| Worker: | |
| Worker: | |
| github.com/actions/actions-runner-controller/cmd/ghalistener/listener: | |
| config: | |
| dir: "{{.InterfaceDir}}/mocks" | |
| pkgname: mocks | |
| filename: "{{.InterfaceName|snakecase}}.go" | |
| interfaces: | |
| Client: | |
| Handler: |
| sudo mv kubebuilder /usr/local/ | ||
| go install sigs.k8s.io/controller-runtime/tools/setup-envtest@$(go list -m -f '{{ .Version }}' sigs.k8s.io/controller-runtime | awk -F'[v.]' '{printf "release-%d.%d", $2, $3}') | ||
| ENVTEST_K8S_VERSION=$(go list -m -f '{{ .Version }}' k8s.io/api | awk -F'[v.]' '{printf "1.%d", $3}') | ||
| echo "KUBEBUILDER_ASSETS=$(setup-envtest use ${ENVTEST_K8S_VERSION} -p path)" >> $GITHUB_ENV |
There was a problem hiding this comment.
The GitHub workflow setup-envtest command uses setup-envtest use ${ENVTEST_K8S_VERSION} -p path without the --bin-dir parameter that's used in the Makefile (line 72-75 of Makefile). While this isn't necessarily wrong, it creates an inconsistency between local development and CI. The Makefile explicitly specifies --bin-dir $(GOBIN) to ensure binaries are stored in a known location. Without this parameter in the workflow, setup-envtest will use its default location, which should still work but creates an unnecessary difference between environments.
| echo "KUBEBUILDER_ASSETS=$(setup-envtest use ${ENVTEST_K8S_VERSION} -p path)" >> $GITHUB_ENV | |
| echo "KUBEBUILDER_ASSETS=$(setup-envtest use ${ENVTEST_K8S_VERSION} --bin-dir=$(go env GOBIN) -p path)" >> $GITHUB_ENV |
Kubebuilder deprecated kubebuilder-tools. Read more: https://book.kubebuilder.io/reference/artifacts
This change sets up env test