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

Enable ginkgolinter and fix finding #1689

Merged
merged 3 commits into from
Mar 15, 2024

Conversation

nunnatsa
Copy link
Contributor

Proposed changes

Enable ginkgolinter in .golangci.yaml, and fix its findings.

The fixing was done by running the ginkgolinter cli. All the finding except for one, are categorized as style findings, e.g. Expect(err).To(BeNil()) instead of Expect(err).ToNot(HaveOccurred()).

The linter also found one real issue in tests/suite/longevity_test.go, where there was a check (Expect) with no assertion (To(...)). That was fixed manually from:

Expect(framework.WriteContent(resultsFile, "\n## Traffic\n"))

to

Expect(framework.WriteContent(resultsFile, "\n## Traffic\n")).To(Succeed())

Closes #1678

Checklist

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

None

@nunnatsa nunnatsa requested a review from a team as a code owner March 14, 2024 06:33
@github-actions github-actions bot added the chore Pull requests for routine tasks label Mar 14, 2024
Copy link
Contributor

@sjberman sjberman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

internal/framework/events/loop_test.go Outdated Show resolved Hide resolved
@kate-osborn
Copy link
Contributor

@nunnatsa, thanks for adding this! This will be very helpful 😄

@kate-osborn kate-osborn self-requested a review March 14, 2024 15:58
@nunnatsa nunnatsa force-pushed the chore/enable-ginkgolinter branch from 4bf8cde to 80b2d3b Compare March 15, 2024 08:25
@nunnatsa
Copy link
Contributor Author

@kate-osborn and @sjberman, thanks for the review. Fixed. PTAL.

BTW, the next version of the ginkgolinter will include an option to force Expect().To()/ToNot() + otiopn to auto fix Expect().Should()/ShouldNot().

@sjberman sjberman enabled auto-merge (squash) March 15, 2024 22:58
@sjberman sjberman merged commit 907f74f into nginxinc:main Mar 15, 2024
40 checks passed
@nunnatsa nunnatsa deleted the chore/enable-ginkgolinter branch March 16, 2024 06:53
amimimor pushed a commit to amimimor/nginx-gateway-fabric that referenced this pull request Apr 3, 2024
Enable ginkgolinter in .golangci.yaml, and fix its findings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Pull requests for routine tasks community
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Use a linter gor Ginkgo tests
4 participants