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

refactor: introduce *test packages in lieu of //go:build test #3238

Merged
merged 29 commits into from
Aug 2, 2024
Merged

refactor: introduce *test packages in lieu of //go:build test #3238

Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
efa5a74
refactor: introduce `cache/cachtest`
ARR4N Jul 26, 2024
b843a56
chore: include changes forgotten in last commit
ARR4N Jul 26, 2024
1012187
refactor: introduce `chains/atomic/atomictest`
ARR4N Jul 26, 2024
7d552b8
refactor: introduce `codec/codectest`
ARR4N Jul 26, 2024
34fd483
refactor: introduce `database/dbtest`
ARR4N Jul 26, 2024
ef718a1
refactor: introduce `ids/idstest`
ARR4N Jul 26, 2024
bda8137
fix: `x/merkledb` tests use `database/dbtest`
ARR4N Jul 26, 2024
9a301dd
chore: move dot imports to end to bring attention to them
ARR4N Jul 26, 2024
bd9fea9
chore: fix import ordering and pray to the CI linter gods
ARR4N Jul 26, 2024
0e96fa4
refactor: introduce `wallet/subnet/primary/common/utxotest`
ARR4N Aug 1, 2024
e38117f
refactor: introduce `vms/platformvm/warp/signertest`
ARR4N Aug 1, 2024
82991fd
refactor: introduce `snow/validators/statetest`
ARR4N Aug 1, 2024
3a095e1
refactor: introduce `snow/networking/sender/sendertest`
ARR4N Aug 1, 2024
28f2de9
refactor: move `snow/networking/benchlist` test-only code to test file
ARR4N Aug 1, 2024
049948b
refactor: intrdouce `snow/engine/snowman/block/blocktest`
ARR4N Aug 1, 2024
403501d
refactor: introduce `snow/engine/enginetest`
ARR4N Aug 1, 2024
891e7fe
Merge branch 'master' into arr4n/test-packages-over-tags
ARR4N Aug 1, 2024
7e2f1d7
refactor: introduce `snow/engine/avalanche/vertex/vertextest`
ARR4N Aug 1, 2024
37b9e20
refactor: move `snow/engine/avalanche/bootstrap/queue` test-only code…
ARR4N Aug 1, 2024
9b9a5da
refactor: move `snow/consensus/snowball` test-only code to test file
ARR4N Aug 1, 2024
3c505f9
chore: depend on `coreth` tag compatible with these changes `#monorepo`
ARR4N Aug 1, 2024
52bf535
fix: make `gci` happy
ARR4N Aug 1, 2024
9430f4c
chore: run `go mod tidy`
ARR4N Aug 1, 2024
1377cb4
refactor: alias `codecpkg` in `codectest` to use `codec` as a variable
ARR4N Aug 2, 2024
d44ea82
chore: remove linter exception for `go:build test` tags
ARR4N Aug 2, 2024
968e3ef
chore: rename `statetest` to `validatorstest`
ARR4N Aug 2, 2024
17c63ff
Merge branch 'master' into arr4n/test-packages-over-tags
ARR4N Aug 2, 2024
2a031b2
chore: depend on `coreth` tag compatible with these changes `#monorepo`
ARR4N Aug 2, 2024
3203eac
fix: test merged from `master` that uses new package
ARR4N Aug 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,9 @@ linters-settings:
- standard
- default
- blank
- dot
- prefix(github.com/ava-labs/avalanchego)
- alias
- dot
Copy link
Contributor

Choose a reason for hiding this comment

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

ni: seems can be put back

Copy link
Contributor

Choose a reason for hiding this comment

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

I think having the dot import be the bottom makes sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a deliberate change to make the dot imports more obvious (I think it should be done for "blank" underscore imports too, but that's beyond this PR).

Both dot and underscore imports fundamentally change the way the rest of the code works, but in a way that's easily missed. Having them called out through placement is one way to mitigate this.

skip-generated: true
custom-order: true
gosec:
Expand Down
11 changes: 5 additions & 6 deletions cache/test_cacher.go → cache/cachetest/cacher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

//go:build test

package cache
package cachetest
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there are a number of fields that stutter based on this rename - do we want to rename things like cachetest.TestIntSize to cachetest.IntSize?

If we want to move that into a followup because the change right now is "methodical" whereas this would be a bit more of a "case-by-case" basis... I think that's fine... But I think it should be a fast-follow... Not an amorphous TODO (I'd personally be in-favor of just cleaning them up in this PR)

I think this applies to most (if not all) of the test packages in this refactor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They were deliberately ignored to be allow this PR to have the smallest possible footprint required to remove test tags. I'll do them as an immediate PR (might even be ready before you re-review this one).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(might even be ready before you re-review this one)

I take this back. There are sooooo many of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#3260


import (
"testing"

"github.com/stretchr/testify/require"

"github.com/ava-labs/avalanchego/cache"
"github.com/ava-labs/avalanchego/ids"
)

Expand All @@ -22,13 +21,13 @@ func TestIntSizeFunc(ids.ID, int64) int {
// CacherTests is a list of all Cacher tests
var CacherTests = []struct {
Size int
Func func(t *testing.T, c Cacher[ids.ID, int64])
Func func(t *testing.T, c cache.Cacher[ids.ID, int64])
}{
{Size: 1, Func: TestBasic},
{Size: 2, Func: TestEviction},
}

func TestBasic(t *testing.T, cache Cacher[ids.ID, int64]) {
func TestBasic(t *testing.T, cache cache.Cacher[ids.ID, int64]) {
require := require.New(t)

id1 := ids.ID{1}
Expand Down Expand Up @@ -63,7 +62,7 @@ func TestBasic(t *testing.T, cache Cacher[ids.ID, int64]) {
require.Equal(expectedValue2, value)
}

func TestEviction(t *testing.T, cache Cacher[ids.ID, int64]) {
func TestEviction(t *testing.T, cache cache.Cacher[ids.ID, int64]) {
require := require.New(t)

id1 := ids.ID{1}
Expand Down
9 changes: 6 additions & 3 deletions cache/lru_cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,29 @@
// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package cache
package cache_test

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/ava-labs/avalanchego/cache/cachetest"
"github.com/ava-labs/avalanchego/ids"

. "github.com/ava-labs/avalanchego/cache"
StephenButtolph marked this conversation as resolved.
Show resolved Hide resolved
)

func TestLRU(t *testing.T) {
cache := &LRU[ids.ID, int64]{Size: 1}

TestBasic(t, cache)
cachetest.TestBasic(t, cache)
}
StephenButtolph marked this conversation as resolved.
Show resolved Hide resolved

func TestLRUEviction(t *testing.T) {
cache := &LRU[ids.ID, int64]{Size: 2}

TestEviction(t, cache)
cachetest.TestEviction(t, cache)
}

func TestLRUResize(t *testing.T) {
Expand Down
13 changes: 8 additions & 5 deletions cache/lru_sized_cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,29 @@
// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved.
// See the file LICENSE for licensing terms.

package cache
package cache_test

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/ava-labs/avalanchego/cache/cachetest"
"github.com/ava-labs/avalanchego/ids"

. "github.com/ava-labs/avalanchego/cache"
)

func TestSizedLRU(t *testing.T) {
cache := NewSizedLRU[ids.ID, int64](TestIntSize, TestIntSizeFunc)
cache := NewSizedLRU[ids.ID, int64](cachetest.TestIntSize, cachetest.TestIntSizeFunc)

TestBasic(t, cache)
cachetest.TestBasic(t, cache)
}

func TestSizedLRUEviction(t *testing.T) {
cache := NewSizedLRU[ids.ID, int64](2*TestIntSize, TestIntSizeFunc)
cache := NewSizedLRU[ids.ID, int64](2*cachetest.TestIntSize, cachetest.TestIntSizeFunc)

TestEviction(t, cache)
cachetest.TestEviction(t, cache)
}

func TestSizedLRUWrongKeyEvictionRegression(t *testing.T) {
Expand Down
5 changes: 3 additions & 2 deletions cache/metercacher/cache_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/require"

"github.com/ava-labs/avalanchego/cache"
"github.com/ava-labs/avalanchego/cache/cachetest"
"github.com/ava-labs/avalanchego/ids"
)

Expand All @@ -29,13 +30,13 @@ func TestInterface(t *testing.T) {
{
description: "sized cache LRU",
setup: func(size int) cache.Cacher[ids.ID, int64] {
return cache.NewSizedLRU[ids.ID, int64](size*cache.TestIntSize, cache.TestIntSizeFunc)
return cache.NewSizedLRU[ids.ID, int64](size*cachetest.TestIntSize, cachetest.TestIntSizeFunc)
},
},
}

for _, scenario := range scenarios {
for _, test := range cache.CacherTests {
for _, test := range cachetest.CacherTests {
baseCache := scenario.setup(test.Size)
c, err := New("", prometheus.NewRegistry(), baseCache)
require.NoError(t, err)
Expand Down
Loading
Loading