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

neonvm-controller: introduce unit tests #935

Merged
merged 17 commits into from
Jun 21, 2024
Merged

neonvm-controller: introduce unit tests #935

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
351aba0
add TESTARGS
Omrigan Mar 18, 2024
c22275a
move old tests
Omrigan May 23, 2024
d4c1037
initial test coverage
Omrigan May 14, 2024
955d921
wip
Omrigan May 17, 2024
518d8bd
fix lints
Omrigan May 17, 2024
53db1cf
chace rebase
Omrigan May 23, 2024
2a0db8c
reenable functests
Omrigan May 30, 2024
7155899
delete util.Ptr
Omrigan May 30, 2024
fb8a8c7
Merge branch 'main' into oleg/neonvm-tests
Omrigan May 30, 2024
376f250
change pkg
Omrigan May 30, 2024
37dd6a7
add comment defaultVm
Omrigan May 30, 2024
1968858
Merge branch 'main' into oleg/neonvm-tests
Omrigan Jun 13, 2024
f9a8e8e
use existing fake client implementation
Omrigan Jun 13, 2024
4349c34
Merge branch 'main' into oleg/neonvm-tests
Omrigan Jun 18, 2024
5fec735
ignore coverage html
Omrigan Jun 18, 2024
fdbb58b
changes according to the comments
Omrigan Jun 18, 2024
82a3903
Merge branch 'main' into oleg/neonvm-tests
Omrigan Jun 21, 2024
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
4 changes: 3 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ vet: ## Run go vet against code.
CGO_ENABLED=0 go vet ./...


TESTARGS ?= ./...
.PHONY: test
test: fmt vet envtest ## Run tests.
# chmodding KUBEBUILDER_ASSETS dir to make it deletable by owner,
Expand All @@ -107,7 +108,8 @@ test: fmt vet envtest ## Run tests.
export KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)"; \
find $(KUBEBUILDER_ASSETS) -type d -exec chmod 0755 {} \; ; \
CGO_ENABLED=0 \
go test ./... -coverprofile cover.out
go test $(TESTARGS) -coverprofile cover.out
go tool cover -html=cover.out -o cover.html
Omrigan marked this conversation as resolved.
Show resolved Hide resolved

##@ Build

Expand Down
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ require (
github.com/sirupsen/logrus v1.9.0 // indirect
github.com/spf13/cobra v1.6.1 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/stretchr/objx v0.5.0 // indirect
github.com/vishvananda/netns v0.0.0-20211101163701-50045581ed74 // indirect
go.etcd.io/etcd/api/v3 v3.5.6 // indirect
go.etcd.io/etcd/client/pkg/v3 v3.5.6 // indirect
Expand Down
1 change: 1 addition & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,7 @@ github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
Expand Down
4 changes: 2 additions & 2 deletions neonvm/controllers/suite_test.go → neonvm/controllers/functests/suite_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package controllers
package functests

import (
"path/filepath"
Expand Down Expand Up @@ -51,7 +51,7 @@ var _ = BeforeSuite(func() {

By("bootstrapping test environment")
testEnv = &envtest.Environment{
CRDDirectoryPaths: []string{filepath.Join("..", "config", "crd", "bases")},
CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")},
ErrorIfCRDPathMissing: true,
}

Expand Down
7 changes: 4 additions & 3 deletions neonvm/controllers/vm_controller_test.go → ...ntrollers/functests/vm_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package controllers
package functests
Copy link
Member

Choose a reason for hiding this comment

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

from DMs: if the reason to move this into a separate package is just so that it can be individually tested, that can also be accomplished with go test <file> -- so I'd prefer leaving this as-is unless there's other reasons as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are more than one test file in the controllers/, and it would be possible to run them all, while leaving functests aside.

Copy link
Member

Choose a reason for hiding this comment

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

I still don't quite get what you mean, but if you get value from having them separate, then fair enough, let's go with it :)

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 meant there is vm_controller_test.go and vm_qmp_test.go, so by go test ./neonvm/controllers we can run them both.

Also it is "aesthetically" looks better IMO that lightweight unit tests are in the same folder, but heavier functional tests are in a different folder.


import (
"context"
Expand All @@ -31,6 +31,7 @@ import (
"k8s.io/apimachinery/pkg/types"

vmv1 "github.com/neondatabase/autoscaling/neonvm/apis/neonvm/v1"
"github.com/neondatabase/autoscaling/neonvm/controllers"
)

var _ = Describe("VirtualMachine controller", func() {
Expand Down Expand Up @@ -93,11 +94,11 @@ var _ = Describe("VirtualMachine controller", func() {
}, time.Minute, time.Second).Should(Succeed())

By("Reconciling the custom resource created")
virtualmachineReconciler := &VMReconciler{
virtualmachineReconciler := &controllers.VMReconciler{
Client: k8sClient,
Scheme: k8sClient.Scheme(),
Recorder: nil,
Config: &ReconcilerConfig{
Config: &controllers.ReconcilerConfig{
IsK3s: false,
UseContainerMgr: true,
MaxConcurrentReconciles: 1,
Expand Down
Loading
Loading