Enable testifylint linter rules

[ivanvc]

What would you like to be added?

This issue formalizes @mmorel-35's work (thanks!) to enable the linter rules from the testifylint linter.

The completion track is as follows:

• [x] bool-compare - #18686, #18717
• [x] compares - #18715
• [x] empty - #18712
• [x] error-is-as - #18740
• [x] error-nil - #18716
• [x] expected-actual - #18720
• [ ] float-compare
• [ ] formatter - #18741
• [ ] go-require
• [x] len - #18712
• [x] negative-positive - #18715
• [x] nil-compare - #18689, #18717
• [ ] require-error

Why is this needed?

To improve the code quality.

[ghouscht]

Hey @ivanvc, do you need some help here? I could take on a few items on the list if you want me to. Just let me know which are the preferred ones. Thank you

[ivanvc]

Hi @ghouscht, thanks for offering your help with this task. Although it's unassigned (because I can't assign it to people outside of the etcd-io organization if they haven't participated in the issue), @mmorel-35 is doing it.

Therefore, I'm going to redirect the question. @mmorel-35, is there anything that @ghouscht could do to help with this task? Thanks.

[mmorel-35]

We actually have two open PR for this linter so until they are merged you can start with the error-is-as or formatter with formatter.require-f-funcs option enabled. Keep your PR as draft until one of them is merged so the number of opened PR stay narrowed concerning testifylint.

You can configure it with https://golangci-lint.run/usage/linters/#testifylint

Golangci lint can not fix it with --fix. You can use testifylint with -fix to start fixing it then review the changes to see if everthing works correctly.

[ghouscht]

Thank you for the response @mmorel-35, I found some time today and opened two PRs for the error-is-as and formatter rules:

• https://github.com/etcd-io/etcd/pull/18740
• https://github.com/etcd-io/etcd/pull/18741

I also tagged you on the PRs (sorry for the spam 🙂).

[ivanvc]

/assign @mmorel-35 @ghouscht

[jmhbnz]

Just curious - we should compare the make verify runtime before enabling these checks and now to make sure we aren't increasing test runtimes too significantly.

@mmorel-35 - As you have enabled these linter rules have you noticed any meaningful increase in test runtimes in CI or locally?

[mmorel-35]

I haven't noticed any significant increase. What make you think there could be regarding the nature of changes that has been done yet ?

[ghouscht]

Running make verify-lint yields the following runtime on my machine

with a clean golangci-lint cache:

• with testifylint 21.941s
• without 20.632s

with golangci-lint cache present:

• with testifylint 4.926s
• without 4.947s

I'd say that is neglible 🙂