linter(testifylint): use Len or Empty for arrays testing

[mmorel-35]

Please add a summary of your change

Use testifylint to use Len and Empty in tests when possible
This has been done through

testifylint --disable-all --enable=empty,len -fix ./...

It is then ensured through golangci-lint configuration

Does your change fix a particular issue?

No

Please indicate you've done the following:

• Accepted the DCO. Commits without the DCO will delay acceptance.
• Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
• Updated the corresponding documentation in site/content/docs/main.

[mateusoliveira43]

@mmorel-35 thanks for your work improving the code quality!

A few comments:

• I would reference this issue in the PR Enable more linters for Velero #6023
• to test your changes, I ran make lint command (which is also run by CI). That was not successful because the golangci-lint version the project uses is v1.54.2, and testifylint was added to golangci-lint in version v1.55.0. References:

  • velero/hack/build-image/Dockerfile

    Line 96 in 13f4efd

    RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.54.2

  • https://github.com/golangci/golangci-lint/releases/tag/v1.55.0
• This PR would not enforce that new contributions use Len and Empty in tests, because of this config

  velero/golangci.yaml

  Lines 36 to 37 in 13f4efd

  	skip-files:
  	- ".*_test.go$"

  I already tried to fix this in fix: Clean up golangci-lint config #7194, but unfortunately did not get any reviews or comments from maintainers there

[mmorel-35]

@mateusoliveira43,
Thank you for your review on this. I believe it's better to keep this PR narrow. So I think that including the upgrade of golangci is acceptable but extending the linting to every _test.go files is going to lead to more change so it is probably better to let it happen in a future PR.

WDYT ?

[mateusoliveira43]

@mmorel-35 I agree, the more simple the PR, the better!

Just want to warn maintainers that current project's config is not very reliable

[blackpiglet]

@mmorel-35 I agree, the more simple the PR, the better!

Just want to warn maintainers that current project's config is not very reliable

I agree with your opinion. To enable linter for all test files, would be good, although it doesn't have a high priority.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.70%. Comparing base (13f4efd) to head (f864c2d).
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7555      +/-   ##
==========================================
+ Coverage   61.68%   61.70%   +0.01%     
==========================================
  Files         263      263              
  Lines       28826    28861      +35     
==========================================
+ Hits        17781    17808      +27     
- Misses       9787     9793       +6     
- Partials     1258     1260       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mmorel-35]

I believe this is ready for CI.
There was two errors related to golangci-lint update one from goconst, the other from nonolint

[mmorel-35]

To enable linter for all test files, would be good, although it doesn't have a high priority.

I could give it a try if you are interested ?

[blackpiglet]

@mmorel-35
Sure. Thanks for your contribution.

[mmorel-35]

@blackpiglet ,
Anything else to do to merge this ?

[blackpiglet]

@mateusoliveira43
PR needs two maintainers' approval. Involved more maintainers to take a look.