hack/verify-test-code.sh: replace with proper linter

[pohly]

What would you like to be added?

A proper linter like something built on https://github.com/quasilyte/go-ruleguard may become a more reliable and capable linter for test guidelines.

• usage of framework.ExpectError
• avoid the Expect.Equal(someBoolean, ...) anti-pattern
• gomega annotations support printf-formatting and thus should not be passed the result of fmt.Sprintf

An enhanced forbidigo could detect:

• usage of wait.Poll and friends instead of gomega.Eventually
• usage of gomega instead of the new framework.Gomega inside test/e2e/framework in functions that don't accept a stack offset

import-boss should be used to

• that test/e2e/framework itself remains light-weight and doesn't import sub-packages

Why is this needed?

The script uses grep to enforce certain code patterns. This is prone to false negatives. For example, it missed this until the code was changed to be all in one line

test/e2e_node/resource_collector.go:

                       gomega.Expect(e2epod.WaitForPodToDisappear(f.ClientSet, f.Namespace.Name, pod.ObjectMeta.Name, labels.Everything(),
                                30*time.Second, 10*time.Minute)).NotTo(gomega.HaveOccurred())

The current test/e2e/framework/.import-restrictions is probably out-dated.

[k8s-ci-robot]

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[pohly]

/sig testing
/area e2e-test-framework

[yashsingh74]

@pohly Can I help in finishing the opened task?

[pohly]

@yashsingh74: you can look into updating .import-restrictions. It's too early to start with the others.

@PauloGoncalvesLima was interested in doing something with go-ruleguard.

[PauloGoncalvesLima]

I'm going to start working on it today.

[pohly]

https://github.com/nunnatsa/ginkgolinter is promising. It can enforce the usage of BeTrue and then we can forbid that function to ensure that booleans are checked with plain if.

[PauloGoncalvesLima]

@pohly, Sorry for the delay college was getting me these past weeks. I did some search and had a doubt about how this golint would be implemented. Would the changes be enforced by the Git Actions? or pre-commit? where this change would need to be implemented?

[pohly]

The linter needs to be part of the golangci-lint invocation(s) in hack/verify-golangci-lint.sh. The go-ruleguard page has information on how to use it from golangci-lint. The unsolved problem is writing such rules.

Note that ginkgolinter probably covers quite a few of the things that I planned to do with go-ruleguard.

Let's put this issue on hold, I'll come back to it soonish with an update.

[pohly]

/reopen

[k8s-ci-robot]

@pohly: Reopened this issue.

In response to this:

/reopen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[PauloGoncalvesLima]

@pohly, Ok, if there is an issue that I can help with, please let me know. 🚀

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[pohly]

Still waiting for golangci/golangci-lint#3612 and golangci/golangci-lint#3617.

/remove-lifecycle stale

[pohly]

avoid the #105678 anti-pattern

This can be done through a combination of different linters:

• Don't allow usage of the deprecated framework.Expect[Not]Equal in favor of gomega.Expect. This will be part of strict golangci-lint #117288.
• The already enabled ginkgolinter will prevent the usage of Expect().To(Equal(true)) in favor of Expect().To(BeTrue())).
• Once forbidigo: better support for configuring complex rules golangci/golangci-lint#3612 is merged and enabled in Kubernetes, we can forbid the usage of gomega.BeTrue and gomega.BeFalse.

[Transmitt0r]

Way to go! @pohly is there something that I can already start working on?

[pohly]

@Transmitt0r: it's still in flux and probably to early to work on.

Regarding BeTrue, see also onsi/gomega#670 and #118044 (comment)

[pohly]

/assign

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[pohly]

/remove-lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[pohly]

/remove-lifecycle stale
/lifecycle frozen