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

Jump to bottom

feat: add suit test cases for controller #99

Closed
wants to merge 4 commits into from
Closed

feat: add suit test cases for controller #99

wants to merge 4 commits into from

Conversation

Aisuko
Copy link
Contributor

@Aisuko Aisuko commented May 15, 2023

Closes #

📑 Description

As I mentioned before, this time I add the test cases for the controller. This PR is without the import of any further packages. And all the test cases were passed.

And this PR also updates the CI checking to check run all the test cases.

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

ℹ Additional Information

@Aisuko ➜ /workspaces/k8sgpt-operator (feat/test-cases) $ make test
test -s /workspaces/k8sgpt-operator/bin/controller-gen && /workspaces/k8sgpt-operator/bin/controller-gen --version | grep -q v0.11.1 || \
GOBIN=/workspaces/k8sgpt-operator/bin go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.11.1
/workspaces/k8sgpt-operator/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
/workspaces/k8sgpt-operator/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
go fmt ./...
go vet ./...
test -s /workspaces/k8sgpt-operator/bin/setup-envtest || GOBIN=/workspaces/k8sgpt-operator/bin go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
KUBEBUILDER_ASSETS="/workspaces/k8sgpt-operator/bin/k8s/1.26.0-linux-amd64" go test ./... -coverprofile cover.out
?       github.com/k8sgpt-ai/k8sgpt-operator    [no test files]
ok      github.com/k8sgpt-ai/k8sgpt-operator/api/v1alpha1       0.019s  coverage: 58.0% of statements
?       github.com/k8sgpt-ai/k8sgpt-operator/pkg/client [no test files]
?       github.com/k8sgpt-ai/k8sgpt-operator/pkg/common [no test files]
?       github.com/k8sgpt-ai/k8sgpt-operator/pkg/resources      [no test files]
?       github.com/k8sgpt-ai/k8sgpt-operator/pkg/utils  [no test files]
ok      github.com/k8sgpt-ai/k8sgpt-operator/controllers        64.796s coverage: 25.2% of statements

@Aisuko Aisuko requested review from a team as code owners May 15, 2023 05:07
@Aisuko
feat: add suit test cases for controller
d895391 
Signed-off-by: Aisuko <urakiny@gmail.com>
@Aisuko
Copy link
Contributor Author

Aisuko commented May 15, 2023

Hi, @AlexsJones, please take a look at this PR. It is a useful one.

@AlexsJones
Copy link
Member

@Aisuko I found that these tests will indefinitely hang, looks like something occurring in the shutdown sequence?

KUBEBUILDER_ASSETS="/Users/alex/Code/k8sgpt-operator/bin/k8s/1.26.0-darwin-arm64" go test ./... -coverprofile cover.out
?   	github.com/k8sgpt-ai/k8sgpt-operator	[no test files]
?   	github.com/k8sgpt-ai/k8sgpt-operator/pkg/client	[no test files]
?   	github.com/k8sgpt-ai/k8sgpt-operator/pkg/common	[no test files]
?   	github.com/k8sgpt-ai/k8sgpt-operator/pkg/resources	[no test files]
?   	github.com/k8sgpt-ai/k8sgpt-operator/pkg/utils	[no test files]
ok  	github.com/k8sgpt-ai/k8sgpt-operator/api/v1alpha1	0.259s	coverage: 58.0% of statements

Aisuko
Aisuko commented May 15, 2023
View reviewed changes
controllers/suite_test.go
})

var _ = AfterSuite(func() {
By("tearing down the test environment")
err := testEnv.Stop()
Expect(err).NotTo(HaveOccurred())
// Here we need to exit directly because the testEnv.Stop() may hang forever in some cases.
if err != nil {
Copy link
Contributor Author

@Aisuko Aisuko May 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@AlexsJones Sometimes, testEnv will stop failing and I hit that issue before. See kubernetes-sigs/controller-runtime#1571 (comment) but I believe they have fixed this issue in this version. Is it anlways indefinitely hanging in your env? My testing env is a codespace with 16core 32GB memory and I set the timeout to 3 minutes. I did not hit this issue.

Copy link
Member

Choose a reason for hiding this comment

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

Mine is an M2 pro, so similar spec - shall we test to see if this hangs the GH action?

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 got your point, this PR also change the CI process to make test. As you can see here are several times successful.

https://github.com/k8sgpt-ai/k8sgpt-operator/actions/runs/4976805323/jobs/8905206195?pr=99

KUBEBUILDER_ASSETS="/home/runner/work/k8sgpt-operator/k8sgpt-operator/bin/k8s/1.26.0-linux-amd64" go test ./... -coverprofile cover.out
?   	github.com/k8sgpt-ai/k8sgpt-operator	[no test files]
ok  	github.com/k8sgpt-ai/k8sgpt-operator/api/v1alpha1	0.037s	coverage: 58.0% of statements
?   	github.com/k8sgpt-ai/k8sgpt-operator/pkg/client	[no test files]
?   	github.com/k8sgpt-ai/k8sgpt-operator/pkg/common	[no test files]
?   	github.com/k8sgpt-ai/k8sgpt-operator/pkg/resources	[no test files]
?   	github.com/k8sgpt-ai/k8sgpt-operator/pkg/utils	[no test files]
ok  	github.com/k8sgpt-ai/k8sgpt-operator/controllers	66.830s	coverage: 25.2% of statements

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AlexsJones Can we merge this one? I have tested this PR in my community's operator. It is without any hang.

@arbreezy arbreezy closed this Oct 20, 2023
@Aisuko
Copy link
Contributor Author

Aisuko commented Oct 21, 2023

Hi @arbreezy This PR is not useful anymore right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexsJones AlexsJones AlexsJones approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Aisuko @AlexsJones @arbreezy