Merge pull request #382 from uhafner/newlines

Fix handling of newlines and paragraphs
uhafner · Oct 5, 2024 · e58a830 · e58a830
2 parents 4118c07 + 3cd3c1b
commit e58a830
Show file tree

Hide file tree

Showing 9 changed files with 112 additions and 85 deletions.
diff --git a/src/main/java/edu/hm/hafner/grading/AnalysisMarkdown.java b/src/main/java/edu/hm/hafner/grading/AnalysisMarkdown.java
@@ -34,7 +34,7 @@ protected void createSpecificDetails(final AggregatedScore aggregation, final Li
             final TruncatedStringBuilder details) {
         for (AnalysisScore score : scores) {
             details.addText(getTitle(score, 2))
-                    .addNewline()
+                    .addParagraph()
                     .addText(getPercentageImage(score))
                     .addNewline()
                     .addText(formatColumns("Name", "Reports", "Errors", "High", "Normal", "Low", "Total"))
@@ -78,6 +78,8 @@ protected void createSpecificDetails(final AggregatedScore aggregation, final Li
                         .addText(formatColumns(TOTAL, LEDGER))
                         .addNewline();
             }
+
+            details.addNewline();
         }
     }
 

diff --git a/src/main/java/edu/hm/hafner/grading/AutoGradingRunner.java b/src/main/java/edu/hm/hafner/grading/AutoGradingRunner.java
@@ -215,7 +215,7 @@ protected String createErrorMessageMarkdown(final FilteredLog log) {
         if (log.hasErrors()) {
             var errors = new StringBuilder(ERROR_CAPACITY);
 
-            errors.append("\n## :construction: Error Messages\n```\n");
+            errors.append("\n### :construction: Error Messages\n\n```\n");
             var messages = new StringJoiner("\n");
             log.getErrorMessages().forEach(messages::add);
             errors.append(messages);

diff --git a/src/main/java/edu/hm/hafner/grading/CoverageMarkdown.java b/src/main/java/edu/hm/hafner/grading/CoverageMarkdown.java
@@ -26,7 +26,7 @@ protected void createSpecificDetails(final AggregatedScore aggregation, final Li
             final TruncatedStringBuilder details) {
         for (CoverageScore score : scores) {
             details.addText(getTitle(score, 2))
-                    .addNewline()
+                    .addParagraph()
                     .addText(getImageForScoreOrCoverage(score))
                     .addNewline()
                     .addText(formatColumns("Name", coveredText, missedText))
@@ -61,6 +61,8 @@ protected void createSpecificDetails(final AggregatedScore aggregation, final Li
                         .addText(formatColumns(LEDGER))
                         .addNewline();
             }
+
+            details.addNewline();
         }
     }
 

diff --git a/src/main/java/edu/hm/hafner/grading/FileSystemCoverageReportFactory.java b/src/main/java/edu/hm/hafner/grading/FileSystemCoverageReportFactory.java
@@ -1,11 +1,13 @@
 package edu.hm.hafner.grading;
 
+import java.io.IOException;
 import java.nio.file.Path;
 import java.util.ArrayList;
 
 import org.apache.commons.lang3.StringUtils;
 
 import edu.hm.hafner.analysis.FileReaderFactory;
+import edu.hm.hafner.analysis.ParsingException;
 import edu.hm.hafner.coverage.ContainerNode;
 import edu.hm.hafner.coverage.CoverageParser.ProcessingMode;
 import edu.hm.hafner.coverage.Metric;
@@ -31,9 +33,15 @@ public Node create(final ToolConfiguration tool, final FilteredLog log) {
 
         var nodes = new ArrayList<Node>();
         for (Path file : REPORT_FINDER.find(log, tool)) {
-            var node = parser.parse(new FileReaderFactory(file).create(), file.toString(), log);
-            log.logInfo("- %s: %s", PATH_UTIL.getRelativePath(file), extractMetric(tool, node));
-            nodes.add(node);
+            var factory = new FileReaderFactory(file);
+            try (var reader = factory.create()) {
+                var node = parser.parse(reader, file.toString(), log);
+                log.logInfo("- %s: %s", PATH_UTIL.getRelativePath(file), extractMetric(tool, node));
+                nodes.add(node);
+            }
+            catch (IOException exception) {
+                throw new ParsingException(exception);
+            }
         }
 
         if (nodes.isEmpty()) {

diff --git a/src/main/java/edu/hm/hafner/grading/GradingReport.java b/src/main/java/edu/hm/hafner/grading/GradingReport.java
@@ -1,5 +1,6 @@
 package edu.hm.hafner.grading;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.stream.Collectors;
 
@@ -92,18 +93,21 @@ public StringBuilder getSubScoreDetails(final AggregatedScore score) {
 
         summary.append(createPercentage(score));
 
+        var summaries = new ArrayList<String>();
         if (score.hasTests()) {
-            summary.append(TEST_MARKDOWN.createSummary(score));
+            summaries.add(TEST_MARKDOWN.createSummary(score));
         }
         if (score.hasCodeCoverage()) {
-            summary.append(CODE_COVERAGE_MARKDOWN.createSummary(score));
+            summaries.add(CODE_COVERAGE_MARKDOWN.createSummary(score));
         }
         if (score.hasMutationCoverage()) {
-            summary.append(MUTATION_COVERAGE_MARKDOWN.createSummary(score));
+            summaries.add(MUTATION_COVERAGE_MARKDOWN.createSummary(score));
         }
         if (score.hasAnalysis()) {
-            summary.append(ANALYSIS_MARKDOWN.createSummary(score));
+            summaries.add(ANALYSIS_MARKDOWN.createSummary(score));
         }
+        summary.append(String.join(ScoreMarkdown.LINE_BREAK_PARAGRAPH, summaries));
+        summary.append(ScoreMarkdown.PARAGRAPH);
         return summary;
     }
 

diff --git a/src/main/java/edu/hm/hafner/grading/ScoreMarkdown.java b/src/main/java/edu/hm/hafner/grading/ScoreMarkdown.java
@@ -1,5 +1,6 @@
 package edu.hm.hafner.grading;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Locale;
@@ -26,7 +27,9 @@
 abstract class ScoreMarkdown<S extends Score<S, C>, C extends Configuration> {
     protected static final int ICON_SIZE = 18;
     static final String SPACE = "&nbsp;";
-    static final String LINE_BREAK = "\\\n";
+    static final String LINE_BREAK_PARAGRAPH = "\\\n";
+    static final String LINE_BREAK = "\n";
+    static final String PARAGRAPH = "\n\n";
     static final String LEDGER = ":heavy_minus_sign:";
     static final String IMPACT = ":moneybag:";
     static final String TOTAL = ":heavy_minus_sign:";
@@ -36,7 +39,7 @@ abstract class ScoreMarkdown<S extends Score<S, C>, C extends Configuration> {
     static final int CAPACITY = 1024;
 
     private static final int MAX_SIZE = 10_000; // limit the size of the output to this number of characters
-    private static final String TRUNCATION_TEXT = "\n\nToo many test failures. Grading output truncated.";
+    private static final String TRUNCATION_TEXT = "\n\nToo many test failures. Grading output truncated.\n\n";
     private static final int HUNDRED_PERCENT = 100;
 
     private final String type;
@@ -122,11 +125,11 @@ public String createSummary(final AggregatedScore aggregation) {
             return createNotEnabled();
         }
 
-        var summary = new StringBuilder(CAPACITY);
+        var summaries = new ArrayList<String>();
         for (S score : scores) {
-            summary.append(createSummary(score));
+            summaries.add(createSummary(score));
         }
-        return summary.toString();
+        return String.join(LINE_BREAK_PARAGRAPH, summaries);
     }
 
     protected abstract String createSummary(S score);

diff --git a/src/main/java/edu/hm/hafner/grading/TestMarkdown.java b/src/main/java/edu/hm/hafner/grading/TestMarkdown.java
@@ -8,7 +8,6 @@
 
 import edu.hm.hafner.coverage.TestCase;
 import edu.hm.hafner.grading.TruncatedString.TruncatedStringBuilder;
-import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
 /**
  * Renders the test results in Markdown.
@@ -36,7 +35,7 @@ protected void createSpecificDetails(final AggregatedScore aggregation,
             final List<TestScore> scores, final TruncatedStringBuilder details) {
         for (TestScore score : scores) {
             details.addText(getTitle(score, 2))
-                    .addNewline()
+                    .addParagraph()
                     .addText(getPercentageImage(score))
                     .addNewline()
                     .addText(formatColumns("Name", "Reports", "Passed", "Skipped", "Failed", "Total"))
@@ -80,52 +79,51 @@ protected void createSpecificDetails(final AggregatedScore aggregation,
             }
 
             if (score.hasSkippedTests()) {
-                details.addText("### Skipped Test Cases")
-                        .addNewline();
-                score.getSkippedTests().stream()
-                        .map(this::renderSkippedTest)
-                        .forEach(details::addText);
-                details.addNewline();
+                addTestDetails(details, "### Skipped Test Cases", score.getSkippedTests(), this::renderSkippedTest);
             }
 
             if (score.hasFailures()) {
-                details.addText("### Failures").addNewline();
-                score.getFailures().stream()
-                        .map(this::renderFailure)
-                        .forEach(details::addText);
-                details.addNewline();
+                addTestDetails(details, "### Failures", score.getFailures(), this::renderFailure);
             }
+
+            details.addNewline();
         }
     }
 
+    private void addTestDetails(final TruncatedStringBuilder details,
+            final String title, final List<TestCase> testCases, final Function<TestCase, String> renderer) {
+        details.addNewline().addText(title).addNewline();
+        testCases.stream()
+                .map(renderer)
+                .map(s -> LINE_BREAK + s)
+                .forEach(details::addText);
+    }
+
     private String renderSkippedTest(final TestCase issue) {
-        return format("- %s#%s%n", issue.getClassName(), issue.getTestName());
+        return format("- %s#%s", issue.getClassName(), issue.getTestName());
     }
 
-    @SuppressFBWarnings(value = "VA_FORMAT_STRING_USES_NEWLINE",
-            justification = "Output is Unix anyway")
     private String renderFailure(final TestCase issue) {
         return format("__%s:%s__", issue.getClassName(), issue.getTestName())
-                + LINE_BREAK
                 + getMessage(issue)
+                + PARAGRAPH
                 + format("""
-                        <details>
-                          <summary>Stack Trace</summary>
-                        
-                          ```text
-                          %s
-                          ```
-
-                        </details>
-                        """, issue.getDescription())
-                + LINE_BREAK;
+                <details>
+                  <summary>Stack Trace</summary>
+                
+                  ```text
+                  %s
+                  ```
+                
+                </details>
+                """, issue.getDescription());
     }
 
     private String getMessage(final TestCase issue) {
         if (issue.getMessage().isBlank()) {
             return StringUtils.EMPTY;
         }
-        return issue.getMessage() + LINE_BREAK;
+        return LINE_BREAK_PARAGRAPH + issue.getMessage();
     }
 
     private int sum(final TestScore score, final Function<TestScore, Integer> property) {
@@ -136,11 +134,13 @@ private int sum(final TestScore score, final Function<TestScore, Integer> proper
     protected String createSummary(final TestScore score) {
         var summary = new StringBuilder(CAPACITY);
 
-        summary.append(SPACE).append(SPACE)
+        summary.append(SPACE)
+                .append(SPACE)
                 .append(getTitle(score, 0));
         if (score.hasFailures() || score.hasPassedTests() || score.hasSkippedTests()) {
             summary.append(": ")
-                    .append(format("%2d %% successful", Math.round(score.getPassedSize() * 100.0 / score.getTotalSize())));
+                    .append(format("%2d %% successful",
+                            Math.round(score.getPassedSize() * 100.0 / score.getTotalSize())));
             var joiner = new StringJoiner(", ", " (", ")");
             if (score.hasFailures()) {
                 joiner.add(format(":x: %d failed", score.getFailedSize()));
@@ -151,44 +151,8 @@ protected String createSummary(final TestScore score) {
             if (score.hasSkippedTests()) {
                 joiner.add(format(":see_no_evil: %d skipped", score.getSkippedSize()));
             }
-            summary.append(joiner).append(LINE_BREAK);
+            summary.append(joiner);
         }
         return summary.toString();
     }
-
-    /*
-### :sunny: &nbsp; Quality Monitor
-
----
-<img title="Code Coverage: 73%" width="110" height="110"
-        align="left" alt="Code Coverage: 93%"
-        src="https://raw.githubusercontent.com/uhafner/autograding-model/main/percentages/073.svg" />
-
-- :vertical_traffic_light: &nbsp; Tests: 291 tests passed
-   - ✔️ 291 passed
-   - ❌ 2 failed
-   - 🙈 4 ignored
-&nbsp;
----
-<img title="Code Coverage: 93%" width="110" height="110"
-        align="left" alt="Code Coverage: 93%"
-        src="https://raw.githubusercontent.com/uhafner/autograding-model/main/percentages/093.svg" />
-
-- :footprints: &nbsp; Code Coverage: 96% coverage achieved
-  - 〰️ 70 % lines covered (7 missed)
-  - ➰ 80 % branches covered (10 missed)
-  - 〽️ 45 complexity
-
-
----
-- :microscope: &nbsp; Mutation Coverage: 93% mutations killed
-- ☑️ 99% Test strength
----
-- :warning: &nbsp; Style:: No warnings
-- :bug: &nbsp; Bugs: No warnings
-<br/>
-
-Created by [Quality Monitor](https://github.com/uhafner/quality-monitor/releases/tag/v1.6.0) v1.6.0 (#85eae94). More details are shown in the [GitHub Checks Result](https://github.com/jenkinsci/coverage-model/runs/23474192891).
-
-     */
 }
diff --git a/src/main/java/edu/hm/hafner/grading/TruncatedString.java b/src/main/java/edu/hm/hafner/grading/TruncatedString.java
@@ -135,7 +135,7 @@ public TruncatedString build() {
          */
         @CanIgnoreReturnValue
         public TruncatedStringBuilder addText(final String text) {
-            this.chunks.add(text);
+            chunks.add(text);
             return this;
         }
 
@@ -152,7 +152,7 @@ public TruncatedStringBuilder addText(final String text) {
         @CanIgnoreReturnValue
         public TruncatedStringBuilder addTextIf(final String text, final boolean guard) {
             if (guard) {
-                this.chunks.add(text);
+                chunks.add(text);
             }
             return this;
         }
@@ -164,7 +164,18 @@ public TruncatedStringBuilder addTextIf(final String text, final boolean guard)
          */
         @CanIgnoreReturnValue
         public TruncatedStringBuilder addNewline() {
-            this.chunks.add("\n");
+            chunks.add("\n");
+            return this;
+        }
+
+        /**
+         * Adds a paragraph to the builder. A paragraph consists of two newlines.
+         *
+         * @return this builder
+         */
+        @CanIgnoreReturnValue
+        public TruncatedStringBuilder addParagraph() {
+            chunks.add("\n\n");
             return this;
         }
 
@@ -204,6 +215,11 @@ public TruncatedStringBuilder setChunkOnNewlines() {
             this.chunkOnNewlines = true;
             return this;
         }
+
+        @Override
+        public String toString() {
+            return build().toString();
+        }
     }
 
     private static class Joiner implements Collector<String, Joiner.Accumulator, List<String>> {

diff --git a/src/test/java/edu/hm/hafner/grading/TruncatedStringTest.java b/src/test/java/edu/hm/hafner/grading/TruncatedStringTest.java
@@ -14,6 +14,34 @@
 class TruncatedStringTest {
     private static final String MESSAGE = "Truncated";  // length 9
 
+    @ParameterizedTest(name = "chunkOnNewlines={0}, chunkOnChars={1}")
+    @MethodSource("parameters")
+    public void shouldAddNewlines(final boolean chunkOnNewlines, final boolean chunkOnChars) {
+        var builder = createBuilder(chunkOnNewlines);
+
+        builder.addText("Hello").addNewline();
+        assertThat(getRawString(builder)).isEqualTo("Hello\n");
+        assertThat(build(builder, chunkOnChars, 1000)).isEqualTo("Hello\n");
+
+        builder.addText(", world!");
+        assertThat(getRawString(builder)).isEqualTo("Hello\n, world!");
+        assertThat(build(builder, chunkOnChars, 1000)).isEqualTo("Hello\n, world!");
+    }
+
+    @ParameterizedTest(name = "chunkOnNewlines={0}, chunkOnChars={1}")
+    @MethodSource("parameters")
+    public void shouldAddParagraphs(final boolean chunkOnNewlines, final boolean chunkOnChars) {
+        var builder = createBuilder(chunkOnNewlines);
+
+        builder.addText("Hello").addParagraph();
+        assertThat(getRawString(builder)).isEqualTo("Hello\n\n");
+        assertThat(build(builder, chunkOnChars, 1000)).isEqualTo("Hello\n\n");
+
+        builder.addText(", world!");
+        assertThat(getRawString(builder)).isEqualTo("Hello\n\n, world!");
+        assertThat(build(builder, chunkOnChars, 1000)).isEqualTo("Hello\n\n, world!");
+    }
+
     @ParameterizedTest(name = "chunkOnNewlines={0}, chunkOnChars={1}")
     @MethodSource("parameters")
     public void shouldBuildStrings(final boolean chunkOnNewlines, final boolean chunkOnChars) {