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

stlye: use thelper without linting every test #5064

Merged
merged 6 commits into from
May 8, 2023
Merged

stlye: use thelper without linting every test #5064

merged 6 commits into from
May 8, 2023

Conversation

faddat
Copy link
Member

@faddat faddat commented May 3, 2023

What is the purpose of the change

This PR introduces the thelper linter without moving to lint the tests. It also includes a breaking
change to an internal api, newSimulatorState() to make it more consistent the linter specifies that
the testing library be passed as the first parameter.

Brief Changelog

  • introduce thelper and begin each helper function outside of test files with a call to t.Helper()
    or tb.Helper()
  • newSimulatorState now passes tb first, to be consistent
  • changes a call from WithJSONCodec to WithCodec

Testing and Verifying

This change enhanced tests by allowing them to give more feedback to developers in some situations.

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? no
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? yes
  • How is the feature or change documented? (not applicable

@faddat
Copy link
Member Author

faddat commented May 3, 2023

changelog reminder ftw

@faddat faddat added V:state/compatible/backport State machine compatible PR, should be backported A:backport/v15.x backport patches to v15.x branch labels May 3, 2023
@faddat
Copy link
Member Author

faddat commented May 3, 2023

labels: backport but only to v15

@faddat
Copy link
Member Author

faddat commented May 7, 2023

thank you for catching the dupe @p0mvn

@czarcas7ic
Copy link
Member

Thanks for the addition @faddat, merging!

@faddat
Copy link
Member Author

faddat commented May 7, 2023

thanks :D it will make the linting of the tests way easier :)

@faddat faddat added V:state/compatible/no_backport State machine compatible PR, depends on prior breaks and removed V:state/compatible/backport State machine compatible PR, should be backported A:backport/v15.x backport patches to v15.x branch labels May 7, 2023
@p0mvn
Copy link
Member

p0mvn commented May 8, 2023

Resolved conflict in the changelog. @faddat FYI, we shouldn't need a changelog entry for linter runs or updated.

Adding "no-changelog" label instead should be good.

This PR should be good to merge once CI passes. Thanks for the change @faddat

@p0mvn p0mvn merged commit 5e6d539 into main May 8, 2023
@p0mvn p0mvn deleted the thelper-is-great branch May 8, 2023 23:32
pysel pushed a commit that referenced this pull request Jun 6, 2023
* use thelper in golangci-lint without linting every test

* add changelog entry

* Update tests/e2e/containers/containers.go

Co-authored-by: Roman <[email protected]>

---------

Co-authored-by: Roman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:CLI C:simulator Edits simulator or simulations C:x/poolmanager C:x/txfees T:CI V:state/compatible/no_backport State machine compatible PR, depends on prior breaks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants