-
Notifications
You must be signed in to change notification settings - Fork 847
test: add import compliance tests #4537
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
Merged
Merged
Changes from all commits
Commits
Show all changes
39 commits
Select commit
Hold shift + click to select a range
21b5aea
test: add import compliance tests
JonathanOppenheimer 7e33965
test: tweak violation wording
JonathanOppenheimer 404acd5
test: add violation to show failure example
JonathanOppenheimer dec282e
test: remove violation
JonathanOppenheimer 234a039
test: only enforce on vms/evm directory
JonathanOppenheimer 9a39d6b
test: clarify boundaries
JonathanOppenheimer 5017e1e
test: rename test
JonathanOppenheimer e490151
test: clarify comments
JonathanOppenheimer f411817
chore: lint
JonathanOppenheimer 84fbb35
test: add license violation test
JonathanOppenheimer 498915c
test: inline once used type
JonathanOppenheimer f4fbfac
tests: rename libevm test
JonathanOppenheimer 69e8354
chore: fix typo
JonathanOppenheimer be5228a
test: remove licensing test (excessive)
JonathanOppenheimer d607b78
Merge branch 'master' into JonathanOppenheimer/import-testing
JonathanOppenheimer 99ed2ce
test: check both evm code locations
JonathanOppenheimer e9cc48b
test: move import tests to evm directory
JonathanOppenheimer cbcd663
test: reconsider boundary tests
JonathanOppenheimer f482da4
Update vms/evm/imports_test.go
JonathanOppenheimer c4b31fc
test: use Arran's rewritten test
JonathanOppenheimer 7d513d4
fix: add .go
JonathanOppenheimer 40e5904
Merge branch 'master' into JonathanOppenheimer/import-testing
JonathanOppenheimer b6541d1
chore: lint
JonathanOppenheimer 70f33bc
chore: move comment
JonathanOppenheimer 57f083c
docs: document functions
JonathanOppenheimer 63a88d9
Update vms/evm/imports_test.go
JonathanOppenheimer 249e7cd
Update vms/evm/imports_test.go
JonathanOppenheimer fee0356
Update vms/evm/imports_test.go
JonathanOppenheimer d24963b
test: use absolute file paths
JonathanOppenheimer 7578a3f
Merge branch 'master' into JonathanOppenheimer/import-testing
JonathanOppenheimer 892dc3b
test: remove outdated / unneeded import tests
JonathanOppenheimer 247e2d7
Merge branch 'master' into JonathanOppenheimer/import-testing
JonathanOppenheimer 6f76cfa
test: enforce way graft/evm import direction
JonathanOppenheimer 74c3da1
Update vms/evm/imports_test.go
JonathanOppenheimer a180d44
fix: maru feedback
JonathanOppenheimer e3d87f0
fix: austin feedback
JonathanOppenheimer d4164ec
fix: cleanup boolean logic
JonathanOppenheimer f252545
Merge branch 'master' into JonathanOppenheimer/import-testing
JonathanOppenheimer 0a94664
Merge remote-tracking branch 'origin/master' into JonathanOppenheimer…
JonathanOppenheimer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Oops, something went wrong.
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| // Copyright (C) 2019-2025, Ava Labs, Inc. All rights reserved. | ||
maru-ava marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // See the file LICENSE for licensing terms. | ||
|
|
||
| package evm | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "go/parser" | ||
| "go/token" | ||
| "io/fs" | ||
| "path/filepath" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| const ( | ||
| corethImport = "github.com/ava-labs/avalanchego/graft/coreth" | ||
| subnetEVMImport = "github.com/ava-labs/avalanchego/graft/subnet-evm" | ||
| ) | ||
|
|
||
| // TestImportViolations ensures proper import rules: | ||
| // - graft/coreth can be imported anywhere EXCEPT vms/evm (but vms/evm/emulate is an exception) | ||
maru-ava marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // - graft/subnet-evm can only be imported within graft/subnet-evm itself and vms/evm/emulate | ||
| // - graft/evm can NOT import from graft/coreth or graft/subnet-evm | ||
JonathanOppenheimer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // | ||
| // The rationale for these rules are as follows: | ||
| // | ||
| // coreth can be imported in AvalancheGo, because it was already imported by Avalanche prior to | ||
| // grafting | ||
| // | ||
| // coreth can NOT be imported in the vms/evm package, because the goal is that vms/evm should | ||
| // only contain the 'clean' properly uplifted code, that meets AvalancheGo quality standards. | ||
| // | ||
| // subnet-evm can NOT be imported anywhere in AvalancheGo besides graft/subnet-evm itself, | ||
| // because it must not become a direct dependency of AvalancheGo or mix directly with coreth. | ||
| // | ||
| // graft/evm is the shared code that both coreth and subnet-evm will depend on, so it must not | ||
| // import from either of them to avoid a circular dependency. | ||
| // | ||
| // both coreth and subnet-evm can be imported in the vms/evm/emulate package, because it | ||
| // allows consumers to use both coreth and subnet-evm registration at the same time. | ||
| // | ||
| // TODO(jonathanoppenheimer): remove the graft functionality once the graft package is removed. | ||
| func TestImportViolations(t *testing.T) { | ||
| const root = "../.." | ||
| repoRoot, err := filepath.Abs(root) | ||
| require.NoError(t, err) | ||
|
|
||
| graftDir := filepath.Join(repoRoot, "graft") | ||
JonathanOppenheimer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| graftEVMDir := filepath.Join(graftDir, "evm") | ||
| graftSubnetEVMDir := filepath.Join(graftDir, "subnet-evm") | ||
| vmsEVMDir := filepath.Join(repoRoot, "vms", "evm") | ||
JonathanOppenheimer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| emulateDir := filepath.Join(vmsEVMDir, "emulate") | ||
|
|
||
| var violations []string | ||
JonathanOppenheimer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| err = filepath.Walk(root, func(file string, _ fs.FileInfo, err error) error { | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if strings.ToLower(filepath.Ext(file)) != ".go" { | ||
| return nil | ||
| } | ||
|
|
||
| absFile, err := filepath.Abs(file) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| node, err := parser.ParseFile(token.NewFileSet(), file, nil, parser.ImportsOnly) | ||
| if err != nil { | ||
| return fmt.Errorf("parser.ParseFile(..., %q, ...): %w", file, err) | ||
| } | ||
|
|
||
| inGraftEVM := strings.HasPrefix(absFile, graftEVMDir) | ||
| inGraftSubnetEVM := strings.HasPrefix(absFile, graftSubnetEVMDir) | ||
| inEmulate := strings.HasPrefix(absFile, emulateDir) | ||
| inVMsEVM := strings.HasPrefix(absFile, vmsEVMDir) | ||
|
|
||
| for _, spec := range node.Imports { | ||
| if spec.Path == nil { | ||
| continue | ||
| } | ||
| importPath := strings.Trim(spec.Path.Value, `"`) | ||
|
|
||
| importsCoreth := isImportIn(importPath, corethImport) | ||
| importsSubnetEVM := isImportIn(importPath, subnetEVMImport) | ||
|
|
||
| hasViolation := (importsCoreth && inVMsEVM && !inEmulate || // vm/evm can't import coreth | ||
| importsSubnetEVM && !inGraftSubnetEVM && !inEmulate || // vm/evm can't import subnet-evm | ||
| (importsCoreth || importsSubnetEVM) && inGraftEVM) // graft/evm can't import coreth or subnet-evm | ||
| if hasViolation { | ||
| violations = append(violations, fmt.Sprintf("File %q imports %q", file, importPath)) | ||
| } | ||
| } | ||
| return nil | ||
| }) | ||
|
|
||
| require.NoErrorf(t, err, "filepath.Walk(%q)", root) | ||
| require.Empty(t, violations, "import violations found") | ||
| } | ||
|
|
||
| func isImportIn(importPath, targetedImport string) bool { | ||
| if importPath == targetedImport { | ||
| return true | ||
| } | ||
|
|
||
| // importPath must be at least len(targetedImport) + 1 to be a subpackage | ||
| // (e.g., "github.com/foo/x" is len("github.com/foo") + 2, minimum subpackage length) | ||
| if len(importPath) < len(targetedImport)+1 { | ||
| return false | ||
| } | ||
|
|
||
| // Check if importPath is a subpackage by ensuring it has the targetedImport prefix | ||
| // AND the next character is '/'. This is to prevent false positives where one | ||
| // package name is a prefix of another like "github.com/foo" vs "github.com/foobar". | ||
JonathanOppenheimer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return strings.HasPrefix(importPath, targetedImport) && importPath[len(targetedImport)] == '/' | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.