[BUG] `Eventually` which uses no `Should` does not get auto-fixed

[rmohr]

Describe the bug

There is an Eventually which uses no Should:

                        Eventually(func(g Gomega) {
                                vm, err = virtClient.VirtualMachine(vm.Namespace).Get(vm.Name, &k8smetav1.GetOptions{})
                                g.Expect(err).ToNot(HaveOccurred())
                                g.Expect(controller.HasFinalizer(vm, v1.VirtualMachineControllerFinalizer)).To(BeFalse())
                        }, 2*time.Minute, 1*time.Second)

It gets reported like this:

$ ginkgolinter ./...
/home/rmohr/gerrit/kubevirt/tests/vm_test.go:2290:4: ginkgo-linter: "Eventually": missing assertion method. Expected "Should()" or "ShouldNot()"
/home/rmohr/gerrit/kubevirt/tests/vm_test.go:2302:4: ginkgo-linter: "Eventually": missing assertion method. Expected "Should()" or "ShouldNot()"
/home/rmohr/gerrit/kubevirt/tests/vm_test.go:2311:4: ginkgo-linter: "Eventually": missing assertion method. Expected "Should()" or "ShouldNot()"

Example run:

rmohr@rmohr-kubevirt ~/gerrit/kubevirt/tests (dev)$ ginkgolinter -fix ./...
/home/rmohr/gerrit/kubevirt/tests/vm_test.go:2290:4: ginkgo-linter: "Eventually": missing assertion method. Expected "Should()" or "ShouldNot()"
/home/rmohr/gerrit/kubevirt/tests/vm_test.go:2302:4: ginkgo-linter: "Eventually": missing assertion method. Expected "Should()" or "ShouldNot()"
/home/rmohr/gerrit/kubevirt/tests/vm_test.go:2311:4: ginkgo-linter: "Eventually": missing assertion method. Expected "Should()" or "ShouldNot()"
rmohr@rmohr-kubevirt ~/gerrit/kubevirt/tests (dev)$ ginkgolinter ./...
/home/rmohr/gerrit/kubevirt/tests/vm_test.go:2290:4: ginkgo-linter: "Eventually": missing assertion method. Expected "Should()" or "ShouldNot()"
/home/rmohr/gerrit/kubevirt/tests/vm_test.go:2302:4: ginkgo-linter: "Eventually": missing assertion method. Expected "Should()" or "ShouldNot()"
/home/rmohr/gerrit/kubevirt/tests/vm_test.go:2311:4: ginkgo-linter: "Eventually": missing assertion method. Expected "Should()" or "ShouldNot()"

However when running the linter with -fix, it does not get fixed.

To Reproduce Steps to reproduce the behavior:

1. Create an Eventually which uses inner assertions only, without any Should
2. apply the linter with -fix
3. It gets not fixed

Expected behavior It would be great if that could be automatically fixed as well. Or is this even something to report? Maybe it should check the function signature as well?

[nunnatsa]

Thanks @rmohr

This is on purpose and it also documented here: https://github.com/nunnatsa/ginkgolinter#missing-assertion-method

The reason for that is that the linter can't guess the right test logic. In this case I want the user to decide how assert the test case.

I think that default handling like Should(Succeed()) may not always be the right choice, and if the linter will fix the warning with such default handling, the user may miss it.

[rmohr]

Makes sense. :+1: