Skip to content

Commit

Permalink
Refactoring of TestReportHandler and TreePrinter for Improved Code Qu…
Browse files Browse the repository at this point in the history
…ality and performance optimization (#57)

* Refactoring TestReportHandler.java

This commit refactors the TestReportHandler class to enhance its concurrency handling and simplify its code structure. These changes do not introduce any new functionality but focus on improving the overall performance, readability, and maintainability of the code.

Key Changes:
Concurrent Collection Usage:

Replaced synchronizedMap(new HashMap<>()) with ConcurrentHashMap for classNames, classEntries, and testEntries.
Why: ConcurrentHashMap is more efficient for concurrent operations, reducing the potential for thread contention and improving performance in multi-threaded environments.
Streamlined Collection Initialization:

Simplified the initialization and population of the classEntries map using computeIfAbsent and direct add() method instead of the previous computeIfPresent.
Why: This reduces the complexity of the code, making it more straightforward and easier to maintain.
Optimized String Handling:

Refactored the hasNestedTests(ReportEntry reportEntry) method to use anyMatch instead of filter followed by count.
Why: anyMatch is more efficient for this use case as it short-circuits when a match is found, improving the method’s performance and readability.
General Code Cleanup:

Adjusted import statements and fixed minor formatting issues, such as spacing and line breaks, to adhere to coding standards.

* Small refactoring in the TreePrinter class
  • Loading branch information
neewrobert authored Aug 27, 2024
1 parent 6839304 commit dbab2d4
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,24 @@

import org.apache.maven.surefire.api.report.ReportEntry;

import java.util.*;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.BiFunction;

import static java.util.Collections.*;
import static java.util.Collections.singleton;
import static java.util.Collections.singletonList;

public class TestReportHandler {

protected static final Map<String, Set<String>> classNames = synchronizedMap(new HashMap<>());
protected static final Map<String, List<WrappedReportEntry>> classEntries = synchronizedMap(new HashMap<>());
protected static final Map<String, List<WrappedReportEntry>> testEntries = synchronizedMap(new HashMap<>());
protected static final Map<String, Set<String>> classNames = new ConcurrentHashMap<>();
protected static final Map<String, List<WrappedReportEntry>> classEntries = new ConcurrentHashMap<>();
protected static final Map<String, List<WrappedReportEntry>> testEntries = new ConcurrentHashMap<>();
protected static final int $ = 36;

private final ReportEntry report;
Expand All @@ -34,7 +42,7 @@ public void prepare() {
}
}

public void print(BiFunction<List<WrappedReportEntry>,List<WrappedReportEntry>, TreePrinter> getTreePrinter) {
public void print(BiFunction<List<WrappedReportEntry>, List<WrappedReportEntry>, TreePrinter> getTreePrinter) {
if (isMarkedAsNestedTest()) {
prepareEntriesForNestedTests();
if (isNestedTestReadyToPrint()) {
Expand All @@ -50,8 +58,7 @@ private boolean isMarkedAsNestedTest() {
}

private void prepareClassEntriesForNestedTest() {
classEntries.putIfAbsent(sourceRootName, new ArrayList<>());
classEntries.computeIfPresent(sourceRootName, addToCollection((WrappedReportEntry) report));
classEntries.computeIfAbsent(sourceRootName, k -> new ArrayList<>()).add((WrappedReportEntry) report);
}

private List<WrappedReportEntry> getClassEntryList() {
Expand Down Expand Up @@ -137,9 +144,6 @@ private boolean hasNestedTests(TestSetStats testSetStats) {
}

private boolean hasNestedTests(ReportEntry reportEntry) {
return reportEntry.getSourceName()
.chars()
.filter(c -> c == $)
.count() > 0;
return reportEntry.getSourceName().chars().anyMatch(c -> c == $);
}
}
Original file line number Diff line number Diff line change
@@ -1,17 +1,3 @@
package org.apache.maven.plugin.surefire.report;

import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
import static org.apache.maven.plugin.surefire.report.TestSetStats.concatenateWithTestGroup;
import static org.apache.maven.plugin.surefire.report.TextFormatter.abbreviateName;
import static org.apache.maven.surefire.shared.utils.StringUtils.isBlank;
import static org.apache.maven.surefire.shared.utils.logging.MessageUtils.buffer;

import java.io.IOException;
import java.util.List;
import java.util.Set;
import java.util.stream.LongStream;

/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
Expand All @@ -31,10 +17,24 @@
* under the License.
*/

package org.apache.maven.plugin.surefire.report;

import org.apache.maven.plugin.surefire.log.api.ConsoleLogger;
import org.apache.maven.surefire.shared.lang3.StringUtils;
import org.apache.maven.surefire.shared.utils.logging.MessageBuilder;

import java.io.IOException;
import java.util.List;
import java.util.Set;
import java.util.stream.LongStream;

import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
import static org.apache.maven.plugin.surefire.report.TestSetStats.concatenateWithTestGroup;
import static org.apache.maven.plugin.surefire.report.TextFormatter.abbreviateName;
import static org.apache.maven.surefire.shared.utils.StringUtils.isBlank;
import static org.apache.maven.surefire.shared.utils.logging.MessageUtils.buffer;

/**
* Tree view printer.
*
Expand Down Expand Up @@ -77,22 +77,22 @@ public void printTests() {
testSetStats
.stream()
.map(TestPrinter::new)
.forEach(TestPrinter::printTest);
testSetStats
.stream()
.map(TestPrinter::new)
.forEach(TestPrinter::printDetails);
.forEach(printer -> {
printer.printTest();
printer.printDetails();
});
}

private class TestPrinter {

private final WrappedReportEntry testResult;
private final int treeLength;
private final Theme theme = options.getTheme();
private final Theme theme;

public TestPrinter(WrappedReportEntry testResult) {
this.testResult = testResult;
this.treeLength = getTreeLength();
this.theme = options.getTheme();
}

private void printDetails() {
Expand Down Expand Up @@ -138,7 +138,7 @@ private void printSuccess() {
println(buffer()
.success(theme.successful() + abbreviateName(testResult.getReportName())));
}

private void printSkipped() {
println(buffer()
.warning(theme.skipped() + getSkippedReport())
Expand Down Expand Up @@ -175,27 +175,26 @@ private void printStdOut() {
println(buffer().strong("Standard out").toString());
try {
testResult.getStdout().writeTo(System.out);
} catch (final IOException ignored) {
}
catch (final IOException ignored) {}
}

private void printStdErr() {
println("");
println(buffer().strong("Standard error").toString());
try {
testResult.getStdErr().writeTo(System.err);
} catch (final IOException ignored) {
}
catch (final IOException ignored) {}
}

private void printStackTrace() {
println("");
println(buffer().strong("Stack trace").toString());
String stackTrace = testResult.getStackTrace(false);
if (stackTrace != null && !StringUtils.isBlank(stackTrace)) {
println(testResult.getStackTrace(false));
}
else {
} else {
println("[No stack trace available]");
}
}
Expand Down

0 comments on commit dbab2d4

Please sign in to comment.