Skip to content

Commit

Permalink
Rename IsTestPath to IsTestPackage (#271)
Browse files Browse the repository at this point in the history
This is actually operating on package names, and we only ever give it
package names.

This is sigificant because package names always have / as path
separators, whereas on Windows, paths use \ as path separators.

This code should always be operating on /s. The code already assumed as
much, so be more clear in the code and tests around it that this is what
we're doing.
  • Loading branch information
illicitonion authored May 7, 2024
1 parent dac29ee commit 355cd5e
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 26 deletions.
13 changes: 7 additions & 6 deletions java/gazelle/private/java/java.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,22 @@ import (
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/types"
)

// IsTestPath tries to detect if the directory would contain test files of not.
func IsTestPath(dir string) bool {
if strings.HasPrefix(dir, "javatests/") {
// IsTestPackage tries to detect if the directory would contain test files of not.
// It assumes dir is a forward-slashed package name, not a possibly-back-slashed filepath.
func IsTestPackage(pkg string) bool {
if strings.HasPrefix(pkg, "javatests/") {
return true
}

if strings.Contains(dir, "src/") {
afterSrc := strings.SplitAfterN(dir, "src/", 2)[1]
if strings.Contains(pkg, "src/") {
afterSrc := strings.SplitAfterN(pkg, "src/", 2)[1]
firstDir := strings.Split(afterSrc, "/")[0]
if strings.HasSuffix(strings.ToLower(firstDir), "test") {
return true
}
}

return strings.Contains(dir, "/test/")
return strings.Contains(pkg, "/test/")
}

// This list was derived from a script along the lines of:
Expand Down
35 changes: 16 additions & 19 deletions java/gazelle/private/java/java_test.go
Original file line number Diff line number Diff line change
@@ -1,31 +1,28 @@
package java

import (
"path/filepath"
"testing"
)

func TestIsTestPath(t *testing.T) {
func TestIsTestPackage(t *testing.T) {
tests := map[string]bool{
"": false,
"java/client/src/org/openqa/selenium/WebDriver.java": false,
"java/client/test/org/openqa/selenium/WebElementTest.java": true,
"java/com/example/platform/githubhook/GithubHookModule.java": false,
"javatests/com/example/platform/githubhook/GithubHookModuleTest.java": true,
"project1/src/main/java/com/example/myproject/App.java": false,
"project1/src/test/java/com/example/myproject/TestApp.java": true,
"project1/src/integrationTest/more/Things.java": true,
"src/main/java/com/example/myproject/App.java": false,
"src/test/java/com/example/myproject/TestApp.java": true,
"src/main/com/example/perftest/Query.java": false,
"src/main/com/example/perftest/TestBase.java": false,
"test-utils/src/main/com/example/project/App.java": false,
"": false,
"java/client/src/org/openqa/selenium": false,
"java/client/test/org/openqa/selenium": true,
"java/com/example/platform/githubhook": false,
"javatests/com/example/platform/githubhook": true,
"project1/src/main/java/com/example/myproject": false,
"project1/src/test/java/com/example/myproject": true,
"project1/src/integrationTest/more": true,
"src/main/java/com/example/myproject": false,
"src/test/java/com/example/myproject": true,
"src/main/com/example/perftest": false,
"test-utils/src/main/com/example/project": false,
}

for path, want := range tests {
t.Run(path, func(t *testing.T) {
dir := filepath.Dir(path)
if got := IsTestPath(dir); got != want {
for pkg, want := range tests {
t.Run(pkg, func(t *testing.T) {
if got := IsTestPackage(pkg); got != want {
t.Errorf("isTest() = %v, want %v", got, want)
}
})
Expand Down
2 changes: 1 addition & 1 deletion java/gazelle/private/javaparser/javaparser.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (r Runner) ParsePackage(ctx context.Context, in *ParsePackageRequest) (*jav
ImportedPackagesWithoutSpecificClasses: importedPackages,
Mains: mains,
Files: sorted_set.NewSortedSet(in.Files),
TestPackage: java.IsTestPath(in.Rel),
TestPackage: java.IsTestPackage(in.Rel),
PerClassMetadata: perClassMetadata,
}, nil
}

0 comments on commit 355cd5e

Please sign in to comment.