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

Add test scripts for static check #201

Closed
wants to merge 3 commits into from

Conversation

Madhu-1
Copy link
Contributor

@Madhu-1 Madhu-1 commented Jan 4, 2019

This PR adds test scripts to do a static check
4 static checks are added

  • goimport
  • gofmt
  • go vet
  • go test

update Travis to use test scripts

Fix issues found in goimport tool

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 4, 2019
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Madhu-1
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: msau42

If they are not already assigned, you can assign the PR to them by writing /assign @msau42 in a comment when ready.

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 size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 4, 2019
@Madhu-1
Copy link
Contributor Author

Madhu-1 commented Jan 4, 2019

/assign @msau42

@pohly
Copy link
Contributor

pohly commented Jan 4, 2019

Instead of adding such generic infrastructure one-by-one to each repo, can we come up with a strategy for sharing this common build and test logic in a single place?

I started with that in kubernetes-csi/external-attacher#109 for building and currently wait for feedback. Extended testing as in this PR could also be added to it. I just chose external-attacher as example; the goal is to put the common code somewhere and then just copy+configure in different repos.

@pohly
Copy link
Contributor

pohly commented Jan 4, 2019

Regarding the change itself: I've had a good experience recently with gometalinter. It might be a simpler and more consistent way to run a variety of code quality tests.

@Madhu-1
Copy link
Contributor Author

Madhu-1 commented Jan 5, 2019

@pohly let me know if blow script looks good to you

gometalinter --errors --deadline=5m -j 4 --enable="gofmt" --enable="goimports" --debug --vendor ./... |& stdbuf -oL grep "linter took\\|:warning:\\|:error:"
DEBUG: [Jan  5 02:59:21.815] [ineffassign.4]: ineffassign linter took 31.650332ms
DEBUG: [Jan  5 02:59:21.879] [gofmt.2]: gofmt linter took 97.284999ms
DEBUG: [Jan  5 02:59:22.044] [goimports.3]: goimports linter took 259.830511ms
DEBUG: [Jan  5 02:59:24.370] [vet.6]: vet linter took 2.491181797s
DEBUG: [Jan  5 02:59:24.373] [gotypex.8]: gotypex linter took 3.119376ms
DEBUG: [Jan  5 02:59:24.397] [gotypex.9]: gotypex linter took 23.171582ms
DEBUG: [Jan  5 02:59:24.417] [gotypex.10]: gotypex linter took 19.971409ms
DEBUG: [Jan  5 02:59:24.546] [vetshadow.7]: vetshadow linter took 2.501824847s
DEBUG: [Jan  5 02:59:36.137] [gotype.12]: gotype linter took 11.591461115s
DEBUG: [Jan  5 02:59:42.966] [varcheck.5]: varcheck linter took 21.150996911s
DEBUG: [Jan  5 02:59:44.200] [gotype.14]: gotype linter took 1.233663573s
DEBUG: [Jan  5 02:59:44.209] [deadcode.15]: deadcode linter took 9.152371ms
DEBUG: [Jan  5 02:59:44.806] [structcheck.11]: structcheck linter took 20.388637437s
DEBUG: [Jan  5 02:59:44.828] [goconst.17]: goconst linter took 22.243668ms
DEBUG: [Jan  5 02:59:46.776] [gotype.13]: gotype linter took 10.639139933s
DEBUG: [Jan  5 02:59:46.842] [gocyclo.19]: gocyclo linter took 64.950157ms
DEBUG: [Jan  5 02:59:50.318] [interfacer.1]: interfacer linter took 28.543296619s
DEBUG: [Jan  5 02:59:57.974] [errcheck.20]: errcheck linter took 11.13168141s
DEBUG: [Jan  5 02:59:58.151] [golint.22]: golint linter took 177.432412ms
DEBUG: [Jan  5 03:00:07.576] [unconvert.18]: unconvert linter took 22.74751973s
DEBUG: [Jan  5 03:00:08.963] [maligned.16]: maligned linter took 24.753759546s
DEBUG: [Jan  5 03:00:11.346] [gosec.21]: gosec linter took 21.027722816s
DEBUG: [Jan  5 03:00:38.168] [megacheck.23]: megacheck linter took 40.014863177s

@humblec
Copy link
Contributor

humblec commented Jan 6, 2019

Instead of adding such generic infrastructure one-by-one to each repo, can we come up with a strategy for sharing this common build and test logic in a single place?

I agree with @pohly here. Its better to have a common/generic build and test logic in a common place/way.

I started with that in kubernetes-csi/external-attacher#109 for building and currently wait for feedback. Extended testing as in this PR could also be added to it. I just chose external-attacher as example; the goal is to put the common code somewhere and then just copy+configure in different repos.

Instead of copying I would vote for configure it somehow for all the repos.

@pohly
Copy link
Contributor

pohly commented Jan 11, 2019

@humblec @Madhu-1 we discussed how to get those shared files into different repos in the CSI WG standup. Keeping the files outside of the reps is awkward, because then what should happen when a developer types make test in a fresh checkout of the repo?

The solution adopted in kubernetes-csi/external-attacher#109 is to mirror files with git subtree. Please have a look and review that if you are interested.

@pohly
Copy link
Contributor

pohly commented Jan 11, 2019

@Madhu-1 why filter the output of gometalinter? If I remember correctly, the output is concise enough to show all of it, and if invoked like that, a non-zero exit code from gometalinter will not been seen by make (unless we force the use of bash and pipefail).

@Madhu-1
Copy link
Contributor Author

Madhu-1 commented Jan 11, 2019

as the debug flag is enabled in the metalinter it will log everything which is not relevant for us, just filtering out to make sure we catch warning and error message and even debug logs to make Travis happy (sometimes if gometalinter takes more time to run the tests, if its more than 10 minutes Travis will kill the tests as it won't get any output on stdout or stderr)

@Madhu-1 Madhu-1 closed this Jan 11, 2019
@pohly
Copy link
Contributor

pohly commented Jan 24, 2019

@Madhu-1 can you move these additional checks into the new shared https://github.com/kubernetes-csi/csi-release-tools?

You can first commit them in any of the sidecars using the new repo (kubernetes-csi/external-attacher#109 is likely to be the first one), create a PR there and get the changed tested and reviewed. Then later move your changes into a PR against csi-release-tools as explained in the csi-release-tools/README.md and update the PR against the sidecar repo (kubernetes-csi/external-attacher#109 (review)) - I'll probably have to extend the README.md...

@Madhu-1
Copy link
Contributor Author

Madhu-1 commented Jan 24, 2019

@pohly sure I will do that :)

xing-yang added a commit to xing-yang/external-provisioner that referenced this pull request Aug 23, 2022
d24254f6a Merge pull request kubernetes-csi#202 from xing-yang/kind_0.14.0
0faa3fc7b Update to Kind v0.14.0 images
ef4e1b2b4 Merge pull request kubernetes-csi#201 from xing-yang/add_1.24_image
4ddce251b Add 1.24 Kind image
7fe51491d Merge pull request kubernetes-csi#200 from pohly/bump-kubernetes-version
70915a8ea prow.sh: update snapshotter version
31a3f38b7 Merge pull request kubernetes-csi#199 from pohly/bump-kubernetes-version
7577454a7 prow.sh: bump Kubernetes to v1.22.0

git-subtree-dir: release-tools
git-subtree-split: d24254f6aa780bb6ba36a946973ee01df5633f6b
@xing-yang xing-yang mentioned this pull request Aug 23, 2022
kbsonlong pushed a commit to kbsonlong/external-provisioner that referenced this pull request Dec 29, 2023
Add prometheus metrics to CSI external-attacher using new csi-lib-utils library
kbsonlong pushed a commit to kbsonlong/external-provisioner that referenced this pull request Dec 29, 2023
d24254f6 Merge pull request kubernetes-csi#202 from xing-yang/kind_0.14.0
0faa3fc7 Update to Kind v0.14.0 images
ef4e1b2b Merge pull request kubernetes-csi#201 from xing-yang/add_1.24_image
4ddce251 Add 1.24 Kind image
7fe51491 Merge pull request kubernetes-csi#200 from pohly/bump-kubernetes-version
70915a8e prow.sh: update snapshotter version
31a3f38b Merge pull request kubernetes-csi#199 from pohly/bump-kubernetes-version
7577454a prow.sh: bump Kubernetes to v1.22.0

git-subtree-dir: release-tools
git-subtree-split: d24254f6aa780bb6ba36a946973ee01df5633f6b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants