Skip to content

Commit 8c5954b

Browse files
committed
Look for annotations on methods as well as classes
Currently using something like `@FlakyTest` only works if a whole class is annotated. It can be preferable to just annotate the single flaky method, rather than the whole class. This allows that.
1 parent e3e6b43 commit 8c5954b

13 files changed

Lines changed: 196 additions & 33 deletions

File tree

java/gazelle/generate.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,13 @@ func addFilteringOutOwnPackage(to *sorted_set.SortedSet[types.PackageName], from
415415

416416
func accumulateJavaFile(cfg *javaconfig.Config, testJavaFiles, testHelperJavaFiles *sorted_set.SortedSet[javaFile], separateTestJavaFiles map[javaFile]separateJavaTestReasons, file javaFile, perClassMetadata map[string]java.PerClassMetadata, log zerolog.Logger) {
417417
if cfg.IsJavaTestFile(filepath.Base(file.pathRelativeToBazelWorkspaceRoot)) {
418-
annotationClassNames := perClassMetadata[file.ClassName().FullyQualifiedClassName()].AnnotationClassNames
418+
annotationClassNames := sorted_set.NewSortedSet[string](nil)
419+
metadataForClass := perClassMetadata[file.ClassName().FullyQualifiedClassName()]
420+
annotationClassNames.AddAll(metadataForClass.AnnotationClassNames)
421+
for _, key := range metadataForClass.MethodAnnotationClassNames.Keys() {
422+
annotationClassNames.AddAll(metadataForClass.MethodAnnotationClassNames.Values(key))
423+
}
424+
419425
perFileAttrs := make(map[string]bzl.Expr)
420426
wrapper := ""
421427
for _, annotationClassName := range annotationClassNames.SortedSlice() {

java/gazelle/lang.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ func (l javaLang) Loads() []rule.LoadInfo {
177177
for _, name := range s.Keys() {
178178
loads = append(loads, rule.LoadInfo{
179179
Name: name,
180-
Symbols: s.Values(name),
180+
Symbols: s.SortedValues(name),
181181
})
182182
}
183183
return loads

java/gazelle/private/java/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ go_library(
1212
# Allow visibility for plugins like Kotlin that don't live in this repo
1313
visibility = ["//visibility:public"],
1414
deps = [
15+
"//java/gazelle/private/sorted_multiset",
1516
"//java/gazelle/private/sorted_set",
1617
"//java/gazelle/private/types",
1718
],

java/gazelle/private/java/package.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package java
22

33
import (
4+
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/sorted_multiset"
45
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/sorted_set"
56
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/types"
67
)
@@ -21,5 +22,6 @@ type Package struct {
2122
}
2223

2324
type PerClassMetadata struct {
24-
AnnotationClassNames *sorted_set.SortedSet[string]
25+
AnnotationClassNames *sorted_set.SortedSet[string]
26+
MethodAnnotationClassNames *sorted_multiset.SortedMultiSet[string, string]
2527
}

java/gazelle/private/javaparser/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ go_library(
99
"//java/gazelle/private/java",
1010
"//java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0:javaparser",
1111
"//java/gazelle/private/servermanager",
12+
"//java/gazelle/private/sorted_multiset",
1213
"//java/gazelle/private/sorted_set",
1314
"//java/gazelle/private/types",
1415
"@com_github_rs_zerolog//:zerolog",

java/gazelle/private/javaparser/javaparser.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/java"
99
pb "github.com/bazel-contrib/rules_jvm/java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0"
1010
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/servermanager"
11+
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/sorted_multiset"
1112
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/sorted_set"
1213
"github.com/bazel-contrib/rules_jvm/java/gazelle/private/types"
1314
"github.com/rs/zerolog"
@@ -64,8 +65,15 @@ func (r Runner) ParsePackage(ctx context.Context, in *ParsePackageRequest) (*jav
6465

6566
perClassMetadata := make(map[string]java.PerClassMetadata, len(resp.GetPerClassMetadata()))
6667
for k, v := range resp.GetPerClassMetadata() {
68+
methodAnnotationClassNames := sorted_multiset.NewSortedMultiSet[string, string]()
69+
for method, perMethod := range v.GetPerMethodMetadata() {
70+
for _, annotation := range perMethod.AnnotationClassNames {
71+
methodAnnotationClassNames.Add(method, annotation)
72+
}
73+
}
6774
metadata := java.PerClassMetadata{
68-
AnnotationClassNames: sorted_set.NewSortedSet(v.GetAnnotationClassNames()),
75+
AnnotationClassNames: sorted_set.NewSortedSet(v.GetAnnotationClassNames()),
76+
MethodAnnotationClassNames: methodAnnotationClassNames,
6977
}
7078
perClassMetadata[k] = metadata
7179
}

java/gazelle/private/javaparser/proto/gazelle/java/javaparser/v0/javaparser.proto

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,13 @@ message Package {
5151
}
5252

5353
message PerClassMetadata {
54-
repeated string annotation_class_names = 1;
54+
repeated string annotation_class_names = 1;
55+
// Not all methods will be present here, only those with something interesting to report.
56+
map<string, PerMethodMetadata> per_method_metadata = 2;
57+
}
58+
59+
message PerMethodMetadata {
60+
repeated string annotation_class_names = 1;
5561
}
5662

5763
service Lifecycle {

java/gazelle/testdata/attribute_setting/src/test/com/example/myproject/BUILD.out

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,22 @@ java_test_suite(
1515
],
1616
)
1717

18+
java_junit5_test(
19+
name = "MixedTest",
20+
srcs = ["MixedTest.java"],
21+
flaky = True,
22+
test_class = "com.example.myproject.MixedTest",
23+
runtime_deps = [
24+
"@maven//:org_junit_jupiter_junit_jupiter_engine",
25+
"@maven//:org_junit_platform_junit_platform_launcher",
26+
"@maven//:org_junit_platform_junit_platform_reporting",
27+
],
28+
deps = [
29+
"//src/main/com/example/annotation",
30+
"@maven//:org_junit_jupiter_junit_jupiter_api",
31+
],
32+
)
33+
1834
java_junit5_test(
1935
name = "RandomTest",
2036
srcs = ["RandomTest.java"],
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package com.example.myproject;
2+
3+
import com.example.annotation.FlakyTest;
4+
import org.junit.jupiter.api.Test;
5+
6+
import java.util.Random;
7+
8+
import static org.junit.jupiter.api.Assertions.assertEquals;
9+
10+
public class MixedTest {
11+
@Test
12+
@FlakyTest
13+
public void unreliableTest() {
14+
Random random = new Random();
15+
int r = random.nextInt(2);
16+
assertEquals(r, 0);
17+
}
18+
19+
@Test
20+
public void reliableTest() {
21+
Random random = new Random();
22+
int r = random.nextInt(2);
23+
assertTrue(r < 2);
24+
}
25+
}

java/src/com/github/bazel_contrib/contrib_rules_jvm/javaparser/generators/ClasspathParser.java

Lines changed: 77 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@
55
import static javax.lang.model.element.Modifier.STATIC;
66

77
import com.google.common.base.Splitter;
8-
import com.google.common.collect.ImmutableMap;
98
import com.google.common.collect.ImmutableSet;
10-
import com.google.common.collect.ImmutableSortedSet;
119
import com.google.common.collect.Lists;
1210
import com.sun.source.tree.AnnotationTree;
1311
import com.sun.source.tree.ArrayTypeTree;
@@ -31,7 +29,9 @@
3129
import java.util.HashMap;
3230
import java.util.List;
3331
import java.util.Map;
32+
import java.util.Objects;
3433
import java.util.Set;
34+
import java.util.SortedMap;
3535
import java.util.SortedSet;
3636
import java.util.TreeMap;
3737
import java.util.TreeSet;
@@ -57,7 +57,7 @@ public class ClasspathParser {
5757

5858
// Mapping from fully-qualified class-name to class-names of annotations on that class.
5959
// Annotations will be fully-qualified where that's known, and not where not known.
60-
private final Map<String, SortedSet<String>> annotatedClasses = new TreeMap<>();
60+
final Map<String, PerClassData> perClassData = new TreeMap<>();
6161

6262
// get the system java compiler instance
6363
private static final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
@@ -67,6 +67,46 @@ public ClasspathParser() {
6767
// Doesn't need to do anything currently
6868
}
6969

70+
static class PerClassData {
71+
public PerClassData() {
72+
this(new TreeSet<>(), new TreeMap<>());
73+
}
74+
75+
@Override
76+
public String toString() {
77+
return "PerClassData{"
78+
+ "annotations="
79+
+ annotations
80+
+ ", perMethodAnnotations="
81+
+ perMethodAnnotations
82+
+ '}';
83+
}
84+
85+
public PerClassData(
86+
SortedSet<String> annotations, SortedMap<String, SortedSet<String>> perMethodAnnotations) {
87+
this.annotations = annotations;
88+
this.perMethodAnnotations = perMethodAnnotations;
89+
}
90+
91+
final SortedSet<String> annotations;
92+
93+
final SortedMap<String, SortedSet<String>> perMethodAnnotations;
94+
95+
@Override
96+
public boolean equals(Object o) {
97+
if (this == o) return true;
98+
if (o == null || getClass() != o.getClass()) return false;
99+
PerClassData that = (PerClassData) o;
100+
return Objects.equals(annotations, that.annotations)
101+
&& Objects.equals(perMethodAnnotations, that.perMethodAnnotations);
102+
}
103+
104+
@Override
105+
public int hashCode() {
106+
return Objects.hash(annotations, perMethodAnnotations);
107+
}
108+
}
109+
70110
public ImmutableSet<String> getUsedTypes() {
71111
return ImmutableSet.copyOf(usedTypes);
72112
}
@@ -87,15 +127,6 @@ public ImmutableSet<String> getMainClasses() {
87127
return ImmutableSet.copyOf(mainClasses);
88128
}
89129

90-
public ImmutableMap<String, ImmutableSortedSet<String>> getAnnotatedClasses() {
91-
ImmutableMap.Builder<String, ImmutableSortedSet<String>> builder =
92-
ImmutableMap.builderWithExpectedSize(annotatedClasses.size());
93-
for (Map.Entry<String, SortedSet<String>> entry : annotatedClasses.entrySet()) {
94-
builder.put(entry.getKey(), ImmutableSortedSet.copyOf(entry.getValue()));
95-
}
96-
return builder.build();
97-
}
98-
99130
public void parseClasses(Path directory, List<String> files) throws IOException {
100131
StandardJavaFileManager fileManager = compiler.getStandardFileManager(null, null, null);
101132
List<? extends JavaFileObject> objectFiles =
@@ -245,6 +276,20 @@ public Void visitMethod(com.sun.source.tree.MethodTree m, Void v) {
245276
checkFullyQualifiedType(param.getType());
246277
handleAnnotations(param.getModifiers().getAnnotations());
247278
}
279+
280+
for (AnnotationTree annotation : m.getModifiers().getAnnotations()) {
281+
String annotationClassName = annotation.getAnnotationType().toString();
282+
String importedFullyQualified = currentFileImports.get(annotationClassName);
283+
String currentFullyQualifiedClass = fullyQualify(currentPackage, currentClassName);
284+
if (importedFullyQualified != null) {
285+
noteAnnotatedMethod(
286+
currentFullyQualifiedClass, m.getName().toString(), importedFullyQualified);
287+
} else {
288+
noteAnnotatedMethod(
289+
currentFullyQualifiedClass, m.getName().toString(), annotationClassName);
290+
}
291+
}
292+
248293
return super.visitMethod(m, v);
249294
}
250295

@@ -333,10 +378,27 @@ private Set<String> checkFullyQualifiedType(Tree identifier) {
333378

334379
private void noteAnnotatedClass(
335380
String annotatedFullyQualifiedClassName, String annotationFullyQualifiedClassName) {
336-
if (!annotatedClasses.containsKey(annotatedFullyQualifiedClassName)) {
337-
annotatedClasses.put(annotatedFullyQualifiedClassName, new TreeSet<>());
381+
if (!perClassData.containsKey(annotatedFullyQualifiedClassName)) {
382+
perClassData.put(annotatedFullyQualifiedClassName, new PerClassData());
383+
}
384+
perClassData
385+
.get(annotatedFullyQualifiedClassName)
386+
.annotations
387+
.add(annotationFullyQualifiedClassName);
388+
}
389+
390+
private void noteAnnotatedMethod(
391+
String annotatedFullyQualifiedClassName,
392+
String methodName,
393+
String annotationFullyQualifiedClassName) {
394+
if (!perClassData.containsKey(annotatedFullyQualifiedClassName)) {
395+
perClassData.put(annotatedFullyQualifiedClassName, new PerClassData());
396+
}
397+
PerClassData data = perClassData.get(annotatedFullyQualifiedClassName);
398+
if (!data.perMethodAnnotations.containsKey(methodName)) {
399+
data.perMethodAnnotations.put(methodName, new TreeSet<>());
338400
}
339-
annotatedClasses.get(annotatedFullyQualifiedClassName).add(annotationFullyQualifiedClassName);
401+
data.perMethodAnnotations.get(methodName).add(annotationFullyQualifiedClassName);
340402
}
341403

342404
private String fullyQualify(String packageName, String className) {

0 commit comments

Comments
 (0)