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.

Sign up for GitHub

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

Jump to bottom

lint all tests #5127

Closed
wants to merge 158 commits into from
Closed

lint all tests #5127

wants to merge 158 commits into from

Conversation

faddat
Copy link
Member

@faddat faddat commented May 8, 2023 •
edited
Loading

Note

Please don't review/merge till 16 is real good and done. I'll keep this updated as best I can.

Closes: #5109

What is the purpose of the change

This PR makes the Osmosis tests easier to read, understand and work with by enabling golangci-lint on the tests in osmosis.

It was broken off into three PR's for easier review:

After the others above are merged, then this PR really serves to catch stragglers and change the .golangci.yml file so that:

run:
  tests: true

Brief Changelog

  • Ensure that all tests pass the same linters as non-test code.
  • Temporarily disable condtitionals on the linter ci job.

Note:

There is probably a way to make the conditionals work, but my finding was that the linter would always pass, no matter what.

Testing and Verifying

All tests should function exactly the same as they did before this PR.

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? no
  • How is the feature or change documented? not applicable

DongLieu and others added 30 commits April 27, 2023 15:34
@DongLieu
TestJoinSwapExactAmountInConsistency
70cfbdc
@DongLieu
add cases corners
37074cc
@faddat
run golangci-lint and resolve ineffectual assignment to err
f5dc71d
@faddat
go work sync
9fa7369
@faddat
waypoint in "check all errors in tests"
c688011
@faddat
50 error checks remaining
ac5bcd2
@faddat
42 errors remaining to check in tests
3105a8a
@faddat
all tests pass and as many errors as possible have been checked
cbed0c4
@faddat
revert enabling golangci-lint to check tests, as it would always fail…
31327a6 
… afterwards
@faddat
update changelog
2acfd0e
@faddat
use thelper in golangci-lint without linting every test
1e9eb0c
@faddat @p0mvn
Update tests/e2e/containers/containers.go
d624769 
Co-authored-by: Roman <[email protected]>
@faddat
Merge remote-tracking branch 'origin/main' into faddat/check-all-errors
e9c621f
@faddat
thelper for incentives bench test
d616f90
@faddat
unused code in tests
0a59632
@faddat @czarcas7ic
Update x/concentrated-liquidity/position_test.go
8ae270d 
Co-authored-by: Adam Tucker <[email protected]>
@faddat
Merge branch 'main' into faddat/check-all-errors
ddc5487
@faddat
fix forced type assertions
560388c
@faddat
revert change to linter config
b70f80a
@faddat @pysel
Update tests/e2e/e2e_test.go
b2d1fab 
Co-authored-by: Ruslan Akhtariev <[email protected]>
@faddat @pysel
Update x/concentrated-liquidity/pool_test.go
85c7ecb 
Co-authored-by: Ruslan Akhtariev <[email protected]>
@faddat
skip pool = poolI
8e573d7
@faddat
update swap_test.go to test that cl type assertions fail
eb9cc68
@faddat
trial
56c6152
@faddat
we needed pool because it's used to CalcInGivenOut
daa610e
@faddat
Merge branch 'main' into faddat/check-all-errors
93f5581
@faddat
Merge remote-tracking branch 'origin/main' into faddat/check-all-errors
e243555
@faddat
re-add skip state sync
16c6c97
@faddat
Merge branch 'faddat/unused-in-tests' into accumulate
39f6fc4
@faddat
Merge branch 'faddat/forcetypeassert' into accumulate
6bc25aa
@faddat faddat requested a review from AlpinYukseloglu as a code owner August 3, 2023 04:30
@faddat faddat mentioned this pull request Aug 12, 2023
Closed
@faddat
Copy link
Member Author

faddat commented Aug 12, 2023

thanks @pysel -- the unparam linter was super helpful!

@faddat faddat marked this pull request as draft August 12, 2023 11:55
@faddat faddat mentioned this pull request Aug 16, 2023
Closed
6 tasks
@faddat
Copy link
Member Author

faddat commented Aug 16, 2023

closing in favor of #6083

@faddat faddat closed this Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattverse mattverse mattverse left review comments

@stackman27 stackman27 Awaiting requested review from stackman27

@nicolaslara nicolaslara Awaiting requested review from nicolaslara nicolaslara is a code owner

@p0mvn p0mvn Awaiting requested review from p0mvn p0mvn is a code owner

@czarcas7ic czarcas7ic Awaiting requested review from czarcas7ic czarcas7ic is a code owner

@AlpinYukseloglu AlpinYukseloglu Awaiting requested review from AlpinYukseloglu AlpinYukseloglu is a code owner

Assignees
No one assigned
Labels
A:no-changelog C:app-wiring Changes to the app folder C:CLI C:x/concentrated-liquidity C:x/epochs C:x/gamm Changes, features and bugs related to the gamm module. C:x/incentives C:x/lockup C:x/pool-incentives C:x/poolmanager C:x/superfluid C:x/tokenfactory C:x/twap Changes to the twap module C:x/txfees T:build T:CI V:state/compatible/no_backport State machine compatible PR, depends on prior breaks
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

use golangci-lint on tests
4 participants
@faddat @mattverse @czarcas7ic @DongLieu