Skip to content

feature/54-asciiDocReporter-tags#1945

Open
johthor wants to merge 18 commits into
TNG:masterfrom
johthor:feature/54-asciiDocReporter-tags
Open

feature/54-asciiDocReporter-tags#1945
johthor wants to merge 18 commits into
TNG:masterfrom
johthor:feature/54-asciiDocReporter-tags

Conversation

@johthor

@johthor johthor commented May 18, 2025

Copy link
Copy Markdown
Contributor

Create index files for all used JGiven tags.

These pages distinguish between single-valued and multi-valued tags.

These will be used prepare a hierarchical tag overview.

Issue: TNG#54
Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>
Issue: TNG#54
Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>
Issue: TNG#54
Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>
Issue: TNG#54
Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>
Issue: TNG#54
Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>
Issue: TNG#54
Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>
Issue: TNG#54
Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>
Issue: TNG#54
Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>
Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>

# Conflicts:
#	jgiven-core/src/main/java/com/tngtech/jgiven/report/asciidoc/AsciiDocReportGenerator.java
Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>
@hvennekate

Copy link
Copy Markdown
Collaborator

Moinmoin! Many thanks -- in the meantime, I had already opened #1940 to extract the generator into a module of its own... let me check how we can combine the two.

Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>
@hvennekate

Copy link
Copy Markdown
Collaborator

Hi @johthor ! Would you mind rebasing this onto #1940 ? One key point was to move the generator into its own module. (Sorry for causing more trouble here 😬 )

johthor added 6 commits July 11, 2025 22:43
Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>
Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>

# Conflicts:
#	jgiven-asciidoc-report/src/main/java/com/tngtech/jgiven/report/asciidoc/AsciiDocReportGenerator.java
#	jgiven-asciidoc-report/src/main/java/com/tngtech/jgiven/report/asciidoc/HierarchyCalculator.java
#	jgiven-asciidoc-report/src/test/java/com/tngtech/jgiven/report/asciidoc/AsciiDocIntroSnippetGeneratorTest.java
Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>
Also reformat

Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>
Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>
Also optimize some tag prefixes to improve the understanding.

Issue: TNG#54
Signed-off-by: Johannes Thorn <2544827+johthor@users.noreply.github.com>
@johthor johthor force-pushed the feature/54-asciiDocReporter-tags branch from fcf4be9 to 1866f6a Compare October 5, 2025 22:09
@johthor

johthor commented Oct 9, 2025

Copy link
Copy Markdown
Contributor Author

Hey @hvennekate, I had tried to rebase these commits onto #1940 but it got to confusing for me to finish it.

Now after the extraction has been merged I have merged the current master into this branch and adjusted the code where necessary.

I believe that everything is now working as expected.

At a later date I might still add the tag hierarchy tree as it is shown in the HTML report.

@johthor johthor marked this pull request as ready for review October 9, 2025 07:43
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

@RunWith(Parameterized.class)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we have JUnit5 tests?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Addressed in #2211

@l-1squared l-1squared left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Output looks fine
but for some reason the report says "There are 999 tagged scenarios" when no test was executed

}

static String toAsciiDocTagStart(ExecutionStatus executionStatus) {
public static String toAsciiDocStartTag(final String scenarioName) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks eerily similar to what the TagMapper does. is that intended?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Well, TagMapper deals with Tags, while this mapper deals with scenario names and executionStatuses. That being said, I do wonder if we can somehow combine the two aspects...

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looking at this in greater detail, I come to the conclusion that probably ReportBlockConverter is not entirely necessary: it is an Interface in between two inherently "custom plugin" classes restricting the flexibility in custom code.

this.allTags = allTags;
}

Map<String, Map<String, List<String>>> computeGroupedTag() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we potentially return something that is easier to decypher?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

also isn't the groupoing files by their Tag (but then why the double map?)
so would groupScenarioFilesByTag not be a better name?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I... cleaned it up a little bit, but I still have to really understand what is going on in the caller.

tagNames.add(mkTag("Best Tag"));
final long nineMilliseconds = 10_000_000L;
List<Tag> tags = List.of(mkTag("BestTag"));
long nineMilliseconds = 10_000_000L;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why does nine millis start with 10?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Addressed in #2211

@Test
public void convert_scenario_footer_with_multiple_tags() {
// given
List<Tag> tags = List.of(mkTag("BestTag"), mkTag("OtherTag"), mkTag("NicestTag"));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

"OtherTag" must be really intimidated by it fabulous company :D

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

shouldn't this, of all things, have some tests?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Added in #2211

@hvennekate

Copy link
Copy Markdown
Collaborator

Hi @johthor ! Did you have a chance to have a look at @l-1squared 's comments?

@Override
public String convertScenarioFooterBlock(final ExecutionStatus executionStatus) {
return MetadataMapper.toAsciiDocTagEnd(executionStatus);
public String convertScenarioFooterBlock(final String identifier, final ExecutionStatus executionStatus, final List<Tag> tags) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  • reformat

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

private void writeIndexFileForAllTags(final Map<String, Map<String, List<String>>> strings) {
final var tagFiles = strings.entrySet().stream()
.sorted((o1, o2) -> {
final var tag1 = allTags.get(o1.getValue().keySet().stream().findFirst().orElse(""));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This can likely be simplified

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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


List<String> generateIndexSnippet() {
final ArrayList<String> result = new ArrayList<>();
List<String> generateIntroSnippet(final String description) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

refactor?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

See #1945 (comment) -> we could probably introduce some specific wrapper classes

case SCENARIO_PENDING, SOME_STEPS_PENDING -> "status-is-pending";
case ABORTED -> "status-is-aborted";
case FAILED -> "status-is-failed";
};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

default missing?

}

static String toAsciiDocStartTag(final Tag tag) {
return "// tag::" + toAsciiDocTagName(tag) + "[]";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Did I not see this somewhere already?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

See also #1945 (comment)

import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

@RunWith(Parameterized.class)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Addressed in #2211

tagNames.add(mkTag("Best Tag"));
final long nineMilliseconds = 10_000_000L;
List<Tag> tags = List.of(mkTag("BestTag"));
long nineMilliseconds = 10_000_000L;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Addressed in #2211

}

static String toAsciiDocTagStart(ExecutionStatus executionStatus) {
public static String toAsciiDocStartTag(final String scenarioName) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Well, TagMapper deals with Tags, while this mapper deals with scenario names and executionStatuses. That being said, I do wonder if we can somehow combine the two aspects...

}

static String toAsciiDocTagStart(ExecutionStatus executionStatus) {
public static String toAsciiDocStartTag(final String scenarioName) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looking at this in greater detail, I come to the conclusion that probably ReportBlockConverter is not entirely necessary: it is an Interface in between two inherently "custom plugin" classes restricting the flexibility in custom code.

@Test
public void convert_scenario_footer_with_multiple_tags() {
// given
List<Tag> tags = List.of(mkTag("BestTag"), mkTag("OtherTag"), mkTag("NicestTag"));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants