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.

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

test: add tests for args.go #140

Merged

Conversation

rakshitgondwal
Copy link
Member

Adds additional test cases for args.go in args_test.go.

Signed-off-by: Rakshit Gondwal <rakshitgondwal3@gmail.com>
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 29, 2024
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 29, 2024
@rakshitgondwal
Copy link
Member Author

@rjsadow Can you please have a look why the tests are failing, cant figure it out.

@rjsadow
Copy link
Collaborator

rjsadow commented Jan 29, 2024

Understanding where/how this line came to be: https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_hydrophone/140/pull-hydrophone-simple-run/1751961702041128960#1:build-log.txt%3A139. If you also look at line 144, at the end there's a ,[] at the end. That shouldn't be there.

@rjsadow
Copy link
Collaborator

rjsadow commented Jan 29, 2024

I'm guessing we're not handling a case properly when extra-args are empty.

pkg/common/args_test.go Outdated Show resolved Hide resolved
Signed-off-by: Rakshit Gondwal <rakshitgondwal3@gmail.com>
Signed-off-by: Rakshit Gondwal <rakshitgondwal3@gmail.com>
Signed-off-by: Rakshit Gondwal <rakshitgondwal3@gmail.com>
@rakshitgondwal
Copy link
Member Author

Facing a really weird behaviour, the tests here are running properly with the current changes but while trying test this out locally, it is showing an error.

Trying to run: bin/hydrophone --focus 'Simple pod should contain last line of the log'

and getting

23:26:06 INF API endpoint : https://127.0.0.1:44729
23:26:06 INF Server version : version.Info{Major:"1", Minor:"27", GitVersion:"v1.27.3", GitCommit:"25b4e43193bcda6c7328a6d147b1fb73a33f1598", GitTreeState:"clean", BuildDate:"2023-06-15T00:36:28Z", GoVersion:"go1.20.5", Compiler:"gc", Platform:"linux/amd64"}
panic: interface conversion: interface {} is []interface {}, not []string

goroutine 1 [running]:
sigs.k8s.io/hydrophone/pkg/common.ValidateArgs()
        /home/rakshit/Desktop/hydrophone/pkg/common/args.go:62 +0x6f9
sigs.k8s.io/hydrophone/cmd.glob..func1(0xc0001a5600?, {0x1a08856?, 0x4?, 0x1a0885a?})
        /home/rakshit/Desktop/hydrophone/cmd/root.go:64 +0x85
github.com/spf13/cobra.(*Command).execute(0x29309e0, {0xc00011a100, 0x2, 0x2})
        /home/rakshit/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:987 +0xaa3
github.com/spf13/cobra.(*Command).ExecuteC(0x29309e0)
        /home/rakshit/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1115 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
        /home/rakshit/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1039
sigs.k8s.io/hydrophone/cmd.Execute()
        /home/rakshit/Desktop/hydrophone/cmd/root.go:82 +0x1a
main.main()
        /home/rakshit/Desktop/hydrophone/main.go:24 +0xf

Not just with this branch, but facing the same behavior with the main branch.

Any thoughts on this? @rjsadow @reetasingh

@reetasingh
Copy link
Contributor

reetasingh commented Jan 29, 2024

--focus 'Simple pod should contain last line of the log'

strange. not getting error same from main


reeta@Reetas-MacBook-Pro hydrophone % go run main.go --focus 'Simple pod should contain last line of the log'
12:09:05 INF API endpoint : https://127.0.0.1:50108
12:09:05 INF Server version : version.Info{Major:"1", Minor:"24", GitVersion:"v1.24.0", GitCommit:"4ce5a8954017644c5420bae81d72b09b735c21f0", GitTreeState:"clean", BuildDate:"2022-05-19T15:42:59Z", GoVersion:"go1.18.1", Compiler:"gc", Platform:"linux/arm64"}
12:09:05 INF Using conformance image : 'registry.k8s.io/conformance:v1.24.0'
12:09:05 INF Using busybox image : 'registry.k8s.io/e2e-test-images/busybox:1.36.1-1'
12:09:05 INF Test framework will start '1' threads and use verbosity '4'
12:09:05 INF namespace created conformance

12:09:05 INF serviceaccount created conformance-serviceaccount

12:09:05 INF clusterrole created conformance-serviceaccount

12:09:05 INF clusterrolebinding created conformance-serviceaccount-role

12:09:05 INF pod created e2e-conformance-test

2024/01/29 20:09:05 Running command:
Command env: []
Run from directory: 
Executable path: /usr/local/bin/ginkgo
Args (comma-delimited): /usr/local/bin/ginkgo,--focus=Simple pod should contain last line of the log,--skip=,--noColor=true,/usr/local/bin/e2e.test,--,--disable-log-dump,--repo-root=/kubernetes,--provider=skeleton,--report-dir=/tmp/results,--kubeconfig=
2024/01/29 20:09:05 Now listening for interrupts
I0129 20:09:05.957930      25 e2e.go:129] Starting e2e run "5817a123-d977-45cc-a22d-0a40398cc836" on Ginkgo node 1
{"msg":"Test Suite starting","total":1,"completed":0,"skipped":0,"failed":0}
Running Suite: Kubernetes e2e suite

  1. when running from main branch did you run make build again?
  2. the issue would be mostly coming from https://github.com/kubernetes-sigs/hydrophone/blob/main/pkg/common/args.go#L62 if extra-args is not of type []string but of type []interafce{}

@rakshitgondwal
Copy link
Member Author

rakshitgondwal commented Jan 29, 2024

when running from main branch did you run make build again?

Yep.
Screenshot from 2024-01-30 02-04-30

the issue would be mostly coming from https://github.com/kubernetes-sigs/hydrophone/blob/main/pkg/common/args.go#L62 if extra-args is not of type []string but of type []interafce{}

This check should not be running in this case right? as I have not defined any extraArgs.

If there is no extraArgs defined, the value of that flag inside config is [], and in the logic we are checking extraArgs != "", maybe this is what is causing the error?

@reetasingh
Copy link
Contributor

when running from main branch did you run make build again?

Yep. Screenshot from 2024-01-30 02-04-30

the issue would be mostly coming from https://github.com/kubernetes-sigs/hydrophone/blob/main/pkg/common/args.go#L62 if extra-args is not of type []string but of type []interafce{}

This check should not be running in this case right? as I have not defined any extraArgs.

If there is no extraArgs defined, the value of that flag inside config is [], and in the logic we are checking extraArgs != "", maybe this is what is causing the error?

this will help #141

@rakshitgondwal
Copy link
Member Author

/hold wait for #141 to be merged first

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 29, 2024
@reetasingh
Copy link
Contributor

/hold wait for #141 to be merged first

@rakshitgondwal PTAL #141 , added a cleaner solution. Thanks for highlighting the problem

rakshitgondwal and others added 2 commits January 30, 2024 19:38
Signed-off-by: Rakshit Gondwal <rakshitgondwal3@gmail.com>
@rakshitgondwal
Copy link
Member Author

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 30, 2024
@reetasingh
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 30, 2024
@rjsadow
Copy link
Collaborator

rjsadow commented Feb 2, 2024

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rakshitgondwal, rjsadow

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 2, 2024
@k8s-ci-robot k8s-ci-robot merged commit d94c0a9 into kubernetes-sigs:main Feb 3, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants