Issue-868: fix too many parents for tags

Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>
TNG · Jun 1, 2022 · 4db4b8f · 4db4b8f
1 parent d1329d7
commit 4db4b8f
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 48 deletions.
diff --git a/jgiven-core/src/main/java/com/tngtech/jgiven/impl/ScenarioModelBuilder.java b/jgiven-core/src/main/java/com/tngtech/jgiven/impl/ScenarioModelBuilder.java
@@ -504,7 +504,7 @@ private void addTags(ResolvedTags tags) {
         if (reportModel != null) {
             this.reportModel.addTags(tags.getDeclaredTags());
             //The report model needs to declare the parent tags in a tag map, or the tags cannot be displayed.
-            this.reportModel.addTags(tags.getParents());
+            this.reportModel.addTags(tags.getAncestors());
         }
 
         if (scenarioModel != null) {

diff --git a/jgiven-core/src/main/java/com/tngtech/jgiven/impl/tag/ResolvedTags.java b/jgiven-core/src/main/java/com/tngtech/jgiven/impl/tag/ResolvedTags.java
@@ -3,6 +3,7 @@
 import com.tngtech.jgiven.report.model.Tag;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 /**
@@ -15,25 +16,28 @@ public List<Tag> getDeclaredTags() {
         return resolvedTags.stream().map(resolvedTag -> resolvedTag.tag).collect(Collectors.toList());
     }
 
-    public List<Tag> getParents() {
-        return resolvedTags.stream().flatMap(resolvedTag -> resolvedTag.parents.stream()).collect(Collectors.toList());
+    @SuppressWarnings("CheckStyle")
+    public Set<Tag> getAncestors() {
+        return resolvedTags.stream()
+            .flatMap(resolvedTag -> resolvedTag.ancestors.stream())
+            .collect(Collectors.toSet());
     }
 
     public boolean isEmpty() {
         return resolvedTags.isEmpty();
     }
 
     /**
-     * A single tag declared for a scenario and the parents necessary to display it.
+     * A single tag declared for a scenario and the ancestors necessary to display it.
      */
     public static class ResolvedTag {
         public final Tag tag;
-        public final List<Tag> parents;
+        public final List<Tag> ancestors;
 
-        ResolvedTag(Tag tag, List<Tag> parents) {
+        ResolvedTag(Tag tag, List<Tag> ancestors) {
 
             this.tag = tag;
-            this.parents = parents;
+            this.ancestors = ancestors;
         }
     }
 

diff --git a/jgiven-core/src/main/java/com/tngtech/jgiven/impl/tag/TagCreator.java b/jgiven-core/src/main/java/com/tngtech/jgiven/impl/tag/TagCreator.java
@@ -49,14 +49,14 @@ public ResolvedTags toTags(Class<? extends Annotation> annotationClass, String..
             return new ResolvedTags();
         }
 
-        List<Tag> parents = getTags(annotationClass);
+        List<Tag> ancestors = getAllAncestorTags(annotationClass);
         if (values.length > 0) {
             List<Tag> explodedTags = getExplodedTags(Iterables.getOnlyElement(tags), values, null, tagConfig);
             return explodedTags.stream()
-                .map(tag -> new ResolvedTags.ResolvedTag(tag, parents))
+                .map(tag -> new ResolvedTags.ResolvedTag(tag, ancestors))
                 .collect(new TagCollector());
         } else {
-            return ResolvedTags.from(new ResolvedTags.ResolvedTag(Iterables.getOnlyElement(tags), parents));
+            return ResolvedTags.from(new ResolvedTags.ResolvedTag(Iterables.getOnlyElement(tags), ancestors));
         }
     }
 
@@ -72,7 +72,7 @@ public ResolvedTags toTags(Annotation annotation) {
         }
 
         List<Tag> tags = processConfiguredAnnotation(tagConfig, annotation);
-        List<Tag> parents = getTags(annotationType);
+        List<Tag> parents = getAllAncestorTags(annotationType);
         return tags.stream()
             .map(tag -> new ResolvedTags.ResolvedTag(tag, parents))
             .collect(new TagCollector());
@@ -178,39 +178,34 @@ private TagConfiguration fromIsTag(IsTag isTag, Class<? extends Annotation> anno
             .cssClass(isTag.cssClass())
             .color(isTag.color())
             .style(isTag.style())
-            .tags(getTagNames(annotationType))
+            .tags(getNamesOfParentTags(annotationType))
             .href(isTag.href())
             .hrefGenerator(isTag.hrefGenerator())
             .showInNavigation(isTag.showInNavigation())
             .build();
     }
 
-    //TODO consolidate with below
-    //TODO better naming
-    private List<String> getTagNames(Class<? extends Annotation> annotationType) {
-        return Arrays.stream(annotationType.getAnnotations())
-            .filter(a -> a.annotationType().isAnnotationPresent(IsTag.class))
-            .map(this::toTags)
+    private List<String> getNamesOfParentTags(Class<? extends Annotation> annotationType) {
+        return getTagAnnotationsOn(annotationType)
             .flatMap(resolvedTags -> resolvedTags.getDeclaredTags().stream())
             .map(Tag::toIdString)
             .collect(Collectors.toList());
     }
 
-    private List<Tag> getTags(Class<? extends Annotation> annotationType) {
-        List<Tag> allTags = Lists.newArrayList();
-
-        for (Annotation annotation : annotationType.getAnnotations()) {
-            if (annotation.annotationType().isAnnotationPresent(IsTag.class)) {
-                allTags.addAll(toTags(annotation).resolvedTags.stream()
-                    .flatMap(tag -> {
-                        Stream<Tag> tagStream = Stream.of(tag.tag);
-                        return Stream.concat(tagStream, tag.parents.stream());
-                    })
-                    .collect(Collectors.toList()));
-            }
-        }
+    private List<Tag> getAllAncestorTags(Class<? extends Annotation> annotationType) {
+        return getTagAnnotationsOn(annotationType)
+            .flatMap(resolvedTags -> resolvedTags.resolvedTags.stream())
+            .flatMap(tag -> {
+                Stream<Tag> tagStream = Stream.of(tag.tag);
+                return Stream.concat(tagStream, tag.ancestors.stream());
+            })
+            .collect(Collectors.toList());
+    }
 
-        return allTags;
+    private Stream<ResolvedTags> getTagAnnotationsOn(Class<? extends Annotation> annotationType) {
+        return Arrays.stream(annotationType.getAnnotations())
+            .filter(a -> a.annotationType().isAnnotationPresent(IsTag.class))
+            .map(this::toTags);
     }
 
     private List<String> toStringList(Object[] value) {

diff --git a/jgiven-core/src/main/java/com/tngtech/jgiven/report/model/ReportModel.java b/jgiven-core/src/main/java/com/tngtech/jgiven/report/model/ReportModel.java
@@ -137,10 +137,8 @@ public synchronized void addTag(Tag tag) {
         this.tagMap.put(tag.toIdString(), tag);
     }
 
-    public synchronized void addTags(List<Tag> tags) {
-        for (Tag tag : tags) {
-            addTag(tag);
-        }
+    public synchronized void addTags(Iterable<Tag> tags) {
+        tags.forEach(this::addTag);
     }
 
     public synchronized Tag getTagWithId(String tagId) {

diff --git a/jgiven-core/src/test/java/com/tngtech/jgiven/impl/tag/ResolvedTagsTest.java b/jgiven-core/src/test/java/com/tngtech/jgiven/impl/tag/ResolvedTagsTest.java
@@ -18,7 +18,7 @@ public void testResolvedTagsFiltersForDirectTags() {
     @Test
     public void testResolvedTagsFiltersForParents() {
         ResolvedTags underTest = TestTagGenerator.getEnumeratedResolvedTags(5);
-        assertThat(underTest.getParents()).extracting(Tag::getFullType)
+        assertThat(underTest.getAncestors()).extracting(Tag::getFullType)
             .containsExactly("parent1", "parent2", "parent3", "parent4", "parent5");
     }
 }
diff --git a/jgiven-core/src/test/java/com/tngtech/jgiven/impl/tag/TagCreatorTest.java b/jgiven-core/src/test/java/com/tngtech/jgiven/impl/tag/TagCreatorTest.java
@@ -30,7 +30,7 @@ public void addLogInterceptor() {
     @Test
     public void testAnnotationParsing() {
         Tag tag = getOnlyTagFor(AnnotationTestClass.class.getAnnotations()[0]);
-        assertThat(tag.getName()).isEqualTo("AnnotationWithoutValue");
+        assertThat(tag.getName()).isEqualTo(AnnotationWithoutValue.class.getSimpleName());
         assertThat(tag.getValues()).isEmpty();
         assertThat(interceptor.containsLoggingEvent(record -> record.getLevel() == Level.SEVERE))
             .as("Attempt to convert an annotation without value method results in an error log")
@@ -40,7 +40,7 @@ public void testAnnotationParsing() {
     @Test
     public void testAnnotationWithValueParsing() {
         Tag tag = getOnlyTagFor(AnnotationWithSingleValueTestClass.class.getAnnotations()[0]);
-        assertThat(tag.getName()).isEqualTo("AnnotationWithSingleValue");
+        assertThat(tag.getName()).isEqualTo(AnnotationWithSingleValue.class.getSimpleName());
         assertThat(tag.getValues()).containsExactly("testvalue");
     }
 
@@ -55,15 +55,15 @@ public void testAnnotationWithName() {
     @Test
     public void testAnnotationWithIgnoredValueParsing() {
         Tag tag = getOnlyTagFor(AnnotationWithIgnoredValueTestClass.class.getAnnotations()[0]);
-        assertThat(tag.getName()).isEqualTo("AnnotationWithIgnoredValue");
+        assertThat(tag.getName()).isEqualTo(AnnotationWithIgnoredValue.class.getSimpleName());
         assertThat(tag.getValues()).isEmpty();
         assertThat(tag.toIdString()).isEqualTo(AnnotationWithIgnoredValue.class.getName());
     }
 
     @Test
     public void testAnnotationWithoutExplodedArrayParsing() {
         Tag tag = getOnlyTagFor(AnnotationWithoutExplodedArrayValueTestClass.class.getAnnotations()[0]);
-        assertThat(tag.getName()).isEqualTo("AnnotationWithoutExplodedArray");
+        assertThat(tag.getName()).isEqualTo(AnnotationWithoutExplodedArray.class.getSimpleName());
         assertThat(tag.getValues()).containsExactly("foo", "bar");
     }
 
@@ -86,7 +86,7 @@ public void testAnnotationWithParentTag() {
         Tag tag = getOnlyTagFor(AnnotationWithParentTag.class.getAnnotations()[0]);
         assertThat(tag.getTags()).containsAll(Arrays.asList(
             ParentTag.class.getName(),
-            ParentTagWithValue.class.getPackage().getName() + ".ParentTagWithValue-SomeValue")
+            ParentTagWithValue.class.getName() + "-SomeValue")
         );
     }
 
@@ -103,17 +103,23 @@ public void testAnnotationWithArrayParsing() {
 
     @Test
     public void testAllParentsOfTagAreResolved() {
+        String[] expectedNames = Stream.of(TagWithParentTags.class, ParentTag.class, ParentTagWithValue.class)
+            .map(Class::getSimpleName).toArray(String[]::new);
+
+        ResolvedTags resolvedTags = underTest.toTags(TagWithGrandparentTags.class);
+
+        assertThat(resolvedTags.getAncestors()).extracting(Tag::getName).containsExactlyInAnyOrder(expectedNames);
+        assertThat(resolvedTags.getDeclaredTags()).extracting(Tag::getName)
+            .containsExactly(TagWithGrandparentTags.class.getSimpleName());
+    }
+
+    @Test
+    public void testTagConfigurationOnlyRefersToTheTagsSingleParent() {
         ResolvedTags resolvedTags = underTest.toTags(TagWithGrandparentTags.class);
         Stream<String> parents = resolvedTags.getDeclaredTags().stream()
             .map(Tag::getTags)
             .flatMap(List::stream);
         assertThat(parents).containsExactly(TagWithParentTags.class.getName());
-        assertThat(resolvedTags.getDeclaredTags())
-            .extracting(Tag::getName)
-            .containsExactly("TagWithGrandparentTags");
-        assertThat(resolvedTags.getParents())
-            .extracting(Tag::getName)
-            .containsExactlyInAnyOrder("TagWithParentTags", "ParentTag", "ParentTagWithValue");
     }
 
     private Tag getOnlyTagFor(Annotation annotation) {