Skip to content

Commit

Permalink
Remove gofumpt from required lints, directly use it in make format (#…
Browse files Browse the repository at this point in the history
…1318)

## What is the purpose of the change

Closes: #1309 , context is in the issue. TL;DR, remove gofumpt from being required in CI due to barriers to entry / lack of ease of use with default IDE setups.

Its kept in make format, and make format is intended to be ran pre-releases

## Brief change log

Remove gofumpt from being required.

## Testing and Verifying

This change is a trivial rework / code cleanup without any test coverage.

## Documentation and Release Note

  - Does this pull request introduce a new feature or user-facing behavior changes? yes
  - Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? No -- This is not done, as gofumpt was also added in unreleased code, so this is no net change
  - How is the feature or change documented? N/A

(cherry picked from commit ed10116)

# Conflicts:
#	.golangci.yml
#	.vscode/settings.json
#	Makefile
#	x/epochs/types/hooks_test.go
  • Loading branch information
ValarDragon authored and mergify-bot committed Apr 23, 2022
1 parent 17afb0b commit 6a20b57
Show file tree
Hide file tree
Showing 5 changed files with 178 additions and 1 deletion.
51 changes: 51 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,60 @@ linters:
enable:
- errcheck
- gofmt
<<<<<<< HEAD
# - goimports
# - golint
# - maligned
=======
# - gofumpt <- disabled https://github.com/osmosis-labs/osmosis/issues/1309
- goheader
- goimports
# - gomoddirectives <- disables replaces
- gomodguard
- goprintffuncname
# - gosec <- triggers too much for this round and should be re-enabled later
# - gosimple <- later
- govet
# - ifshort <- seems to be of questionable value
- importas
- ineffassign
# - ireturn <- disabled because we want to return interfaces
# - lll <- disabled as it is a bit much. Maybe we have a desired limit?
- makezero
- misspell
- nakedret
# - nestif <- ?
# - nilerr <- needs go 1.18 support
- nilnil
# - nlreturn <- disabled as it doesn't auto-fix something simple and doesn't add anything useful
# - noctx <- needs go 1.18 support
# - nolintlint <- disabled because we use nolint in some places
- paralleltest
# - prealloc <- disabled because it makes simple code complicated
# - predeclared <- later
- promlinter
# - revive <- temporarily disabled while jacob figures out how to make poolId not trigger it.
# - rowserrcheck <- needs go 1.18 support
# - sqlclosecheck <- needs go 1.18 support
# - staticcheck <- later
# - structcheck <- later
# - stylecheck <- enable later, atriggers on underscores in variable names and on poolId
# - tagliatelle <- disabled for defying cosmos idiom
- tenv
- testpackage
# - thelper <- later
# - tparallel <- needs go 1.18 support
- typecheck
- unconvert
# - unparam <- looks for unused parameters (enable later)
# - unused <- looks for unused variables (enable later)
# - varcheck <- later
# - varnamelen <- disabled because defies idiom
# - wastedassign
- whitespace
# - wrapcheck
# - wsl
>>>>>>> ed10116 (Remove gofumpt from required lints, directly use it in make format (#1318))

issues:
exclude-rules:
Expand Down
27 changes: 27 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
{
"tabnine.experimentalAutoImports": true,
"go.lintTool": "golangci-lint",
"go.formatTool": "goimports",
"go.useLanguageServer": true,
"[go.mod]": {
"editor.formatOnSave": true,
"editor.codeActionsOnSave": {
"source.organizeImports": true
}
},
"[go]": {
"editor.snippetSuggestions": "none",
"editor.formatOnSave": true,
"editor.codeActionsOnSave": {
"source.organizeImports": true
}
},
"gopls": {
"local": "github.com/osmosis-labs/osmosis"
},
"files.eol": "\n",
"[proto3]": {
"editor.defaultFormatter": "xaver.clang-format"
},
"clang-format.style": "{BasedOnStyle: Google, IndentWidth: 2, ColumnLimit: 120, AlignConsecutiveAssignments: true, AlignConsecutiveDeclarations: true, SpacesInSquareBrackets: true}"
}
5 changes: 5 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,14 @@ lint:
@go run github.com/golangci/golangci-lint/cmd/golangci-lint run --timeout=10m

format:
<<<<<<< HEAD
find . -name '*.go' -type f -not -path "./vendor*" -not -path "*.git*" -not -path "./client/lcd/statik/statik.go" | xargs gofmt -w -s
find . -name '*.go' -type f -not -path "./vendor*" -not -path "*.git*" -not -path "./client/lcd/statik/statik.go" | xargs misspell -w
find . -name '*.go' -type f -not -path "./vendor*" -not -path "*.git*" -not -path "./client/lcd/statik/statik.go" | xargs goimports -w -local github.com/cosmos/cosmos-sdk
=======
@go run github.com/golangci/golangci-lint/cmd/golangci-lint run ./... --fix
@go run mvdan.cc/gofumpt -l -w .
>>>>>>> ed10116 (Remove gofumpt from required lints, directly use it in make format (#1318))

###############################################################################
### Localnet ###
Expand Down
95 changes: 95 additions & 0 deletions x/epochs/types/hooks_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package types_test

import (
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/suite"

"github.com/osmosis-labs/osmosis/v7/app/apptesting"
"github.com/osmosis-labs/osmosis/v7/x/epochs/types"
)

type KeeperTestSuite struct {
apptesting.KeeperTestHelper

queryClient types.QueryClient
}

func TestKeeperTestSuite(t *testing.T) {
suite.Run(t, new(KeeperTestSuite))
}

func (suite *KeeperTestSuite) SetupTest() {
suite.Setup()

suite.queryClient = types.NewQueryClient(suite.QueryHelper)
}

// dummyEpochHook is a struct satisfying the epoch hook interface,
// that maintains a counter for how many times its been succesfully called,
// and a boolean for whether it should panic during its execution.
type dummyEpochHook struct {
successCounter int
shouldPanic bool
}

func (hook *dummyEpochHook) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) {
if hook.shouldPanic {
panic("dummyEpochHook is panicking")
}
hook.successCounter += 1
}

func (hook *dummyEpochHook) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochNumber int64) {
if hook.shouldPanic {
panic("dummyEpochHook is panicking")
}
hook.successCounter += 1
}

func (hook *dummyEpochHook) Clone() *dummyEpochHook {
newHook := dummyEpochHook{shouldPanic: hook.shouldPanic, successCounter: hook.successCounter}
return &newHook
}

var _ types.EpochHooks = &dummyEpochHook{}

func (suite *KeeperTestSuite) TestHooksPanicRecovery() {
panicHook := dummyEpochHook{shouldPanic: true}
noPanicHook := dummyEpochHook{shouldPanic: false}
simpleHooks := []dummyEpochHook{panicHook, noPanicHook}

tests := []struct {
hooks []dummyEpochHook
expectedCounterValues []int
}{
{[]dummyEpochHook{noPanicHook}, []int{1}},
{simpleHooks, []int{0, 1}},
}

for tcIndex, tc := range tests {
for epochActionSelector := 0; epochActionSelector < 2; epochActionSelector++ {
suite.SetupTest()
hookRefs := []types.EpochHooks{}

for _, hook := range tc.hooks {
hookRefs = append(hookRefs, hook.Clone())
}

hooks := types.NewMultiEpochHooks(hookRefs...)
suite.NotPanics(func() {
if epochActionSelector == 0 {
hooks.BeforeEpochStart(suite.Ctx, "id", 0)
} else if epochActionSelector == 1 {
hooks.AfterEpochEnd(suite.Ctx, "id", 0)
}
})

for i := 0; i < len(hooks); i++ {
epochHook := hookRefs[i].(*dummyEpochHook)
suite.Require().Equal(tc.expectedCounterValues[i], epochHook.successCounter, "test case index %d", tcIndex)
}
}
}
}
1 change: 0 additions & 1 deletion x/incentives/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ func (suite *KeeperTestSuite) TestGRPCActiveGaugesPerDenom() {
StartTime: startTime,
}
suite.Require().Equal(res.Data[0].String(), expectedGauge.String())

}

func (suite *KeeperTestSuite) TestGRPCUpcomingGauges() {
Expand Down

0 comments on commit 6a20b57

Please sign in to comment.