From 543163573e580be6242783dd613d071fb6cd8a2a Mon Sep 17 00:00:00 2001 From: Jan Scheible Date: Sat, 28 May 2022 17:48:01 +0200 Subject: [PATCH] #33 "source code style rule definition" - all static helper classes are now called *Utils - all other service-style classes have no static methods anymore and use constructor dependency injection --- README.md | 9 + .../maven/AbstractTestGapMojo.java | 4 +- .../maven/DebugCoverageResolutionMojo.java | 6 +- .../maven/TestGapAnalysisMojo.java | 8 +- test-gap-analysis/spotbugs-exclude.xml | 4 +- .../DebugCoverageResolution.java | 24 ++- .../testgapanalysis/TestGapAnalysis.java | 33 ++-- .../testgapanalysis/analysis/Analysis.java | 19 ++- .../common/{Files2.java => FilesUtils.java} | 5 +- ...vaMethodUtil.java => JavaMethodUtils.java} | 6 +- .../testgapanalysis/git/GitDiffer.java | 149 +++++++++++++++++ .../git/{GitHelper.java => GitUtils.java} | 24 +-- .../testgapanalysis/git/RepositoryStatus.java | 155 ++---------------- ...oCoHelper.java => JaCoCoReportParser.java} | 12 +- .../jacoco/MethodWithCoverageInfo.java | 6 +- .../jacoco/resolver/CoverageResolver.java | 4 +- ...{JavaParserHelper.java => JavaParser.java} | 15 +- .../parser/{Masker.java => MaskUtils.java} | 5 +- .../testgapanalysis/parser/ParsedMethod.java | 4 +- .../testgapanalysis/parser/ParserUtils.java | 5 +- .../DebugCoverageResolutionTest.java | 14 +- .../testgapanalysis/TestGapAnalysisTest.java | 19 ++- .../analysis/AnalysisTest.java | 11 +- .../{Files2Test.java => FileUtilsTest.java} | 4 +- .../common/JavaMethodUtilTest.java | 8 +- ...{GitHelperTest.java => GitDifferTest.java} | 20 ++- ...rTest.java => JaCoCoReportParserTest.java} | 8 +- .../resolver/AbstractIntegrationTest.java | 5 +- ...serHelperTest.java => JavaParserTest.java} | 8 +- .../{MaskerTest.java => MaskUtilsTest.java} | 11 +- .../parser/TestClassSourceJavaParser.java | 3 +- 31 files changed, 345 insertions(+), 263 deletions(-) rename test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/{Files2.java => FilesUtils.java} (96%) rename test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/{JavaMethodUtil.java => JavaMethodUtils.java} (95%) create mode 100644 test-gap-analysis/src/main/java/com/scheible/testgapanalysis/git/GitDiffer.java rename test-gap-analysis/src/main/java/com/scheible/testgapanalysis/git/{GitHelper.java => GitUtils.java} (83%) rename test-gap-analysis/src/main/java/com/scheible/testgapanalysis/jacoco/{JaCoCoHelper.java => JaCoCoReportParser.java} (90%) rename test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/{JavaParserHelper.java => JavaParser.java} (91%) rename test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/{Masker.java => MaskUtils.java} (96%) rename test-gap-analysis/src/test/java/com/scheible/testgapanalysis/common/{Files2Test.java => FileUtilsTest.java} (78%) rename test-gap-analysis/src/test/java/com/scheible/testgapanalysis/git/{GitHelperTest.java => GitDifferTest.java} (55%) rename test-gap-analysis/src/test/java/com/scheible/testgapanalysis/jacoco/{JaCoCoHelperTest.java => JaCoCoReportParserTest.java} (88%) rename test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/{JavaParserHelperTest.java => JavaParserTest.java} (96%) rename test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/{MaskerTest.java => MaskUtilsTest.java} (79%) diff --git a/README.md b/README.md index 9755daf..d613248 100644 --- a/README.md +++ b/README.md @@ -202,3 +202,12 @@ Uncovered methods: Unresolvable methods (no coverage information available): ... ``` + +## Source code style + +1. avoid static methods for easier testing + 1. exception: (package) private helper methods in classes that are static to enforce functional purity + 1. exception: completely stateless `*Utils` classes with static methods only + 1. utility classes must be `abstract` and have a private default constructor + 1. exception: real constants with names in upper case delimited by underscores +1. `logger` has to be protected final but not static: `protected final Logger logger = LoggerFactory.getLogger(getClass());` diff --git a/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/AbstractTestGapMojo.java b/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/AbstractTestGapMojo.java index 8cbcf9e..3940c16 100644 --- a/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/AbstractTestGapMojo.java +++ b/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/AbstractTestGapMojo.java @@ -1,6 +1,6 @@ package com.scheible.testgapanalysis.maven; -import com.scheible.testgapanalysis.jacoco.JaCoCoHelper; +import com.scheible.testgapanalysis.jacoco.JaCoCoReportParser; import java.io.File; import java.nio.file.Path; @@ -37,7 +37,7 @@ protected Set findRelevantJaCoCoReportFiles() { final Path outputDirAsPath = outputDir.toPath(); final Path testOutputDirAsPath = testOutputDir.toPath(); - JaCoCoHelper.findJaCoCoReportFiles(buildDir).forEach(file -> { + JaCoCoReportParser.findJaCoCoReportFiles(buildDir).forEach(file -> { final Path fileAsPath = file.toPath(); if (!fileAsPath.startsWith(outputDirAsPath) && !fileAsPath.startsWith(testOutputDirAsPath)) { result.add(file); diff --git a/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/DebugCoverageResolutionMojo.java b/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/DebugCoverageResolutionMojo.java index c4f00fc..bba66d3 100644 --- a/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/DebugCoverageResolutionMojo.java +++ b/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/DebugCoverageResolutionMojo.java @@ -2,6 +2,8 @@ import com.scheible.testgapanalysis.DebugCoverageResolution; import com.scheible.testgapanalysis.DebugCoverageResolutionReport; +import com.scheible.testgapanalysis.jacoco.JaCoCoReportParser; +import com.scheible.testgapanalysis.parser.JavaParser; import java.io.File; @@ -23,7 +25,9 @@ public class DebugCoverageResolutionMojo extends AbstractTestGapMojo { @Override public void execute() throws MojoExecutionException, MojoFailureException { if (buildDir.exists()) { - final DebugCoverageResolutionReport report = DebugCoverageResolution.run(baseDir, sourceDir, + final DebugCoverageResolution debugCoverageResolution = new DebugCoverageResolution(new JavaParser(), + new JaCoCoReportParser()); + final DebugCoverageResolutionReport report = debugCoverageResolution.run(baseDir, sourceDir, findRelevantJaCoCoReportFiles()); logReport(report); diff --git a/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/TestGapAnalysisMojo.java b/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/TestGapAnalysisMojo.java index a9d22eb..549065c 100644 --- a/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/TestGapAnalysisMojo.java +++ b/test-gap-analysis-maven-plugin/src/main/java/com/scheible/testgapanalysis/maven/TestGapAnalysisMojo.java @@ -4,6 +4,10 @@ import com.google.gson.GsonBuilder; import com.scheible.testgapanalysis.TestGapAnalysis; import com.scheible.testgapanalysis.TestGapReport; +import com.scheible.testgapanalysis.analysis.Analysis; +import com.scheible.testgapanalysis.git.GitDiffer; +import com.scheible.testgapanalysis.jacoco.JaCoCoReportParser; +import com.scheible.testgapanalysis.parser.JavaParser; import java.io.File; import java.io.IOException; @@ -28,7 +32,9 @@ public class TestGapAnalysisMojo extends AbstractTestGapMojo { @Override public void execute() throws MojoExecutionException, MojoFailureException { if (buildDir.exists()) { - final TestGapReport report = TestGapAnalysis.run(baseDir, findRelevantJaCoCoReportFiles(), + final TestGapAnalysis testGapAnalysis = new TestGapAnalysis(new Analysis(new JavaParser()), + new JaCoCoReportParser(), new GitDiffer()); + final TestGapReport report = testGapAnalysis.run(baseDir, findRelevantJaCoCoReportFiles(), Optional.ofNullable(referenceCommitHash)); logReport(report); diff --git a/test-gap-analysis/spotbugs-exclude.xml b/test-gap-analysis/spotbugs-exclude.xml index 08d9f1c..69664a4 100644 --- a/test-gap-analysis/spotbugs-exclude.xml +++ b/test-gap-analysis/spotbugs-exclude.xml @@ -1,8 +1,8 @@ - - + + diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/DebugCoverageResolution.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/DebugCoverageResolution.java index d09d193..6027f8e 100644 --- a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/DebugCoverageResolution.java +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/DebugCoverageResolution.java @@ -10,12 +10,12 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Stream; -import com.scheible.testgapanalysis.common.Files2; -import com.scheible.testgapanalysis.jacoco.JaCoCoHelper; +import com.scheible.testgapanalysis.common.FilesUtils; +import com.scheible.testgapanalysis.jacoco.JaCoCoReportParser; import com.scheible.testgapanalysis.jacoco.MethodWithCoverageInfo; import com.scheible.testgapanalysis.jacoco.resolver.CoverageResolver; import com.scheible.testgapanalysis.jacoco.resolver.CoverageResult; -import com.scheible.testgapanalysis.parser.JavaParserHelper; +import com.scheible.testgapanalysis.parser.JavaParser; import com.scheible.testgapanalysis.parser.ParsedMethod; /** @@ -35,20 +35,28 @@ private ParseResult(final Set methods, final int javaFileCount) { } } - public static DebugCoverageResolutionReport run(final File workDir, final File sourceDir, + private final JavaParser javaParser; + private final JaCoCoReportParser jaCoCoReportParser; + + public DebugCoverageResolution(final JavaParser javaParser, final JaCoCoReportParser jaCoCoReportParser) { + this.javaParser = javaParser; + this.jaCoCoReportParser = jaCoCoReportParser; + } + + public DebugCoverageResolutionReport run(final File workDir, final File sourceDir, final Set jaCoCoReportFiles) { - final Set coverageInfo = JaCoCoHelper.getMethodCoverage(jaCoCoReportFiles); + final Set coverageInfo = jaCoCoReportParser.getMethodCoverage(jaCoCoReportFiles); final ParseResult parseResult = parseMethods(sourceDir); final CoverageResolver resolver = CoverageResolver.with(coverageInfo); final CoverageResult result = resolver.resolve(parseResult.methods); - return new DebugCoverageResolutionReport(coverageInfo.size(), Files2.toRelative(workDir, jaCoCoReportFiles), + return new DebugCoverageResolutionReport(coverageInfo.size(), FilesUtils.toRelative(workDir, jaCoCoReportFiles), parseResult.javaFileCount, result.getResolvedMethods(), result.getEmptyMethods(), result.getUnresolvedMethods(), result.getAmbiguousCoverage()); } - private static ParseResult parseMethods(final File workingDir) throws UncheckedIOException { + private ParseResult parseMethods(final File workingDir) throws UncheckedIOException { final AtomicInteger javaFileCount = new AtomicInteger(0); final Set methods = new HashSet<>(); @@ -56,7 +64,7 @@ private static ParseResult parseMethods(final File workingDir) throws UncheckedI walkStream.filter(p -> p.toFile().isFile()).forEach(file -> { if (file.toString().endsWith(".java")) { javaFileCount.incrementAndGet(); - methods.addAll(JavaParserHelper.getMethods(Files2.readUtf8(file.toFile()))); + methods.addAll(javaParser.getMethods(FilesUtils.readUtf8(file.toFile()))); } }); } catch (IOException ex) { diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/TestGapAnalysis.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/TestGapAnalysis.java index 52b1b9d..0e8b250 100644 --- a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/TestGapAnalysis.java +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/TestGapAnalysis.java @@ -18,9 +18,10 @@ import com.scheible.testgapanalysis.TestGapReport.TestGapMethod; import com.scheible.testgapanalysis.analysis.Analysis; import com.scheible.testgapanalysis.analysis.AnalysisResult; -import com.scheible.testgapanalysis.common.Files2; +import com.scheible.testgapanalysis.common.FilesUtils; +import com.scheible.testgapanalysis.git.GitDiffer; import com.scheible.testgapanalysis.git.RepositoryStatus; -import com.scheible.testgapanalysis.jacoco.JaCoCoHelper; +import com.scheible.testgapanalysis.jacoco.JaCoCoReportParser; import com.scheible.testgapanalysis.jacoco.MethodWithCoverageInfo; import com.scheible.testgapanalysis.parser.ParsedMethod; @@ -66,26 +67,36 @@ private CoverageResult(final Set coveredMethods, final Set NON_TEST_JAVA_FILE = f -> f.endsWith(".java") && !f.startsWith("src/test/java/") && !f.contains("/src/test/java/"); - public static TestGapReport run(final File workDir, final Set jaCoCoReportFiles, + private final Analysis analysis; + private final JaCoCoReportParser jaCoCoReportParser; + private final GitDiffer gitDiffer; + + public TestGapAnalysis(final Analysis analysis, final JaCoCoReportParser jaCoCoReportParser, + final GitDiffer gitDiffer) { + this.analysis = analysis; + this.jaCoCoReportParser = jaCoCoReportParser; + this.gitDiffer = gitDiffer; + } + + public TestGapReport run(final File workDir, final Set jaCoCoReportFiles, final Optional referenceCommitHash) { - final Set coverageInfo = JaCoCoHelper.getMethodCoverage(jaCoCoReportFiles); + final Set coverageInfo = jaCoCoReportParser.getMethodCoverage(jaCoCoReportFiles); final RepositoryResult repositoryResult = identifyFileChanges(referenceCommitHash, workDir); final CoverageResult coverageResult = performTestGapAnalysis(repositoryResult.repositoryStatus, coverageInfo); return new TestGapReport(workDir.getAbsolutePath(), repositoryResult.repositoryStatus.getOldCommitHash(), - repositoryResult.repositoryStatus.getNewCommitHash(), Files2.toRelative(workDir, jaCoCoReportFiles), + repositoryResult.repositoryStatus.getNewCommitHash(), FilesUtils.toRelative(workDir, jaCoCoReportFiles), coverageInfo.size(), repositoryResult.newOrChangedFiles, coverageResult.coveredMethods, coverageResult.uncoveredMethods, coverageResult.emptyMethods, coverageResult.unresolvableMethods, coverageResult.ambiguouslyResolvedCoverage); } - private static RepositoryResult identifyFileChanges(final Optional referenceCommitHash, - final File workDir) { + private RepositoryResult identifyFileChanges(final Optional referenceCommitHash, final File workDir) { Set newOrChangedFiles = emptySet(); final RepositoryStatus status = referenceCommitHash - .map(h -> RepositoryStatus.ofCommitComparedToHead(workDir, h)) - .orElseGet(() -> RepositoryStatus.ofWorkingCopyChanges(workDir)); + .map(h -> gitDiffer.ofCommitComparedToHead(workDir, h, NON_TEST_JAVA_FILE)) + .orElseGet(() -> gitDiffer.ofWorkingCopyChanges(workDir, NON_TEST_JAVA_FILE)); if (!(status.getAddedFiles().isEmpty() && status.getChangedFiles().isEmpty())) { newOrChangedFiles = Stream.concat(status.getChangedFiles().stream(), status.getAddedFiles().stream()) @@ -99,9 +110,9 @@ private static RepositoryResult identifyFileChanges(final Optional refer return new RepositoryResult(status, newOrChangedFiles); } - private static CoverageResult performTestGapAnalysis(final RepositoryStatus status, + private CoverageResult performTestGapAnalysis(final RepositoryStatus status, final Set coverageInfo) { - final AnalysisResult result = Analysis.perform(status, NON_TEST_JAVA_FILE, coverageInfo); + final AnalysisResult result = analysis.perform(status, coverageInfo); return new CoverageResult(toTestGapMethods(result.getCoveredMethods()), toTestGapMethods(result.getUncoveredMethods()), toTestGapMethods(result.getEmptyMethods()), diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/analysis/Analysis.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/analysis/Analysis.java index c044246..afae60f 100644 --- a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/analysis/Analysis.java +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/analysis/Analysis.java @@ -12,7 +12,7 @@ import com.scheible.testgapanalysis.jacoco.MethodWithCoverageInfo; import com.scheible.testgapanalysis.jacoco.resolver.CoverageResolver; import com.scheible.testgapanalysis.jacoco.resolver.CoverageResult; -import com.scheible.testgapanalysis.parser.JavaParserHelper; +import com.scheible.testgapanalysis.parser.JavaParser; import com.scheible.testgapanalysis.parser.ParsedMethod; /** @@ -24,18 +24,23 @@ public class Analysis { static final Predicate NON_GETTER_OR_SETTER_METHOD = mcw -> !mcw.getParsedMethod().getName() .startsWith("get") && !mcw.getParsedMethod().getName().startsWith("set"); - public static AnalysisResult perform(final RepositoryStatus status, final Predicate fileFilter, - final Set coverageInfo) { - final Map newOrChangedFilesWithContent = status.getNewContents(fileFilter); + private final JavaParser javaParser; + + public Analysis(final JavaParser javaParser) { + this.javaParser = javaParser; + } + + public AnalysisResult perform(final RepositoryStatus status, final Set coverageInfo) { + final Map newOrChangedFilesWithContent = status.getNewContents(); // all methods of new or changed files in the new state compared to the old state final Set newContentMethods = newOrChangedFilesWithContent.entrySet().stream() - .flatMap(e -> JavaParserHelper.getMethods(e.getValue()).stream()).map(MethodCompareWrapper::new) + .flatMap(e -> javaParser.getMethods(e.getValue()).stream()).map(MethodCompareWrapper::new) .filter(NON_GETTER_OR_SETTER_METHOD).collect(Collectors.toSet()); // all methods of changed files in the new state that already existed in the old state - final Set oldContentMethods = status.getOldContents(fileFilter).entrySet().stream() - .flatMap(e -> JavaParserHelper.getMethods(e.getValue()).stream()).map(MethodCompareWrapper::new) + final Set oldContentMethods = status.getOldContents().entrySet().stream() + .flatMap(e -> javaParser.getMethods(e.getValue()).stream()).map(MethodCompareWrapper::new) .filter(NON_GETTER_OR_SETTER_METHOD).collect(Collectors.toSet()); // @formatter:off diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/Files2.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/FilesUtils.java similarity index 96% rename from test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/Files2.java rename to test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/FilesUtils.java index 57a5ee7..61b1233 100644 --- a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/Files2.java +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/FilesUtils.java @@ -17,12 +17,15 @@ * * @author sj */ -public class Files2 { +public abstract class FilesUtils { private static final String UTF_8_CHARSET = StandardCharsets.UTF_8.name(); private static final String BACKSLASH_PATTERN = "\\\\"; + private FilesUtils() { + } + public static String readUtf8(final Class anchor, final String name) { return readUtf8(anchor.getResourceAsStream(name)); } diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/JavaMethodUtil.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/JavaMethodUtils.java similarity index 95% rename from test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/JavaMethodUtil.java rename to test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/JavaMethodUtils.java index 3a7be3c..ddd186b 100644 --- a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/JavaMethodUtil.java +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/common/JavaMethodUtils.java @@ -13,7 +13,7 @@ * * @author sj */ -public class JavaMethodUtil { +public class JavaMethodUtils { private static final Map PRIMITIVE_TYPE_MAPPING = new HashMap<>(); @@ -33,7 +33,7 @@ public class JavaMethodUtil { * ed.)") and returns Java notations. Java notations means for example '.' separator between nested classes. */ public static List parseDescriptorArguments(final String descriptor) { - return parseArguments(descriptor).stream().map(JavaMethodUtil::convertType).collect(Collectors.toList()); + return parseArguments(descriptor).stream().map(JavaMethodUtils::convertType).collect(Collectors.toList()); } static List parseArguments(final String descriptor) { @@ -137,7 +137,7 @@ private static String removeArrayBrackets(final String type) { } private static List normalizeMethodArguments(final Collection arguments) { - return arguments.stream().map(JavaMethodUtil::normalizeArgument).collect(Collectors.toList()); + return arguments.stream().map(JavaMethodUtils::normalizeArgument).collect(Collectors.toList()); } private static String normalizeArgument(final String argument) { diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/git/GitDiffer.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/git/GitDiffer.java new file mode 100644 index 0000000..b3dcfd4 --- /dev/null +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/git/GitDiffer.java @@ -0,0 +1,149 @@ +package com.scheible.testgapanalysis.git; + +import java.io.File; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.AbstractMap; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.function.Predicate; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.Status; +import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.diff.DiffEntry; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.Repository; + +import com.scheible.testgapanalysis.common.FilesUtils; + +/** + * + * @author sj + */ +public class GitDiffer { + + public RepositoryStatus ofWorkingCopyChanges(final File workingDir, final Predicate fileFilter) { + final Set addedFiles = new HashSet<>(); + final Set changedFiles = new HashSet<>(); + + final Ref head; + File workTreeDir; + + try { + try (Repository repository = GitUtils.open(workingDir)) { + workTreeDir = repository.getWorkTree(); + try (Git git = new Git(repository)) { + head = repository.exactRef(Constants.HEAD); + + final Status status = git.status().call(); + + addedFiles.addAll(status.getAdded()); + addedFiles.addAll(status.getUntracked()); + + changedFiles.addAll(status.getModified()); + changedFiles.removeAll(status.getAdded()); + } catch (GitAPIException ex2) { + throw new IllegalStateException(ex2); + } + } + } catch (IOException ex) { + throw new UncheckedIOException(ex); + } + + return filterFiles(workingDir, workTreeDir, head.getObjectId().getName(), Optional.empty(), addedFiles, + changedFiles, fileFilter); + } + + public RepositoryStatus ofCommitComparedToHead(final File workingDir, final String commitHash, + final Predicate fileFilter) { + return ofCommitsCompared(workingDir, commitHash, Constants.HEAD, fileFilter); + } + + static RepositoryStatus ofCommitsCompared(final File workingDir, final String oldObjectId, final String newObjectId, + final Predicate fileFilter) { + final Set addedFiles = new HashSet<>(); + final Set changedFiles = new HashSet<>(); + + String resolvedNewObjectId; + File workTreeDir; + + try { + try (Repository repository = GitUtils.open(workingDir)) { + workTreeDir = repository.getWorkTree(); + + try (Git git = new Git(repository)) { + resolvedNewObjectId = Constants.HEAD.equals(newObjectId) + ? repository.exactRef(Constants.HEAD).getObjectId().getName() + : newObjectId; + + final List diff = git.diff() + .setOldTree(GitUtils.prepareTreeParser(repository, oldObjectId)) + .setNewTree(GitUtils.prepareTreeParser(repository, resolvedNewObjectId)).call(); + + for (final DiffEntry entry : diff) { + if (entry.getChangeType() == DiffEntry.ChangeType.ADD + || entry.getChangeType() == DiffEntry.ChangeType.COPY) { + addedFiles.add(entry.getNewPath()); + + } else if (entry.getChangeType() == DiffEntry.ChangeType.MODIFY + || entry.getChangeType() == DiffEntry.ChangeType.RENAME) { + changedFiles.add(entry.getNewPath()); + } + } + } catch (GitAPIException ex2) { + throw new IllegalStateException(ex2); + } + } + } catch (IOException ex) { + throw new UncheckedIOException(ex); + } + + return filterFiles(workingDir, workTreeDir, oldObjectId, Optional.of(resolvedNewObjectId), addedFiles, + changedFiles, fileFilter); + } + + /** + * Only include files that are in the working dir or in a sub-directory of working dir. + */ + private static RepositoryStatus filterFiles(final File workingDir, final File workTreeDir, + final String oldCommitHash, final Optional newCommitHash, final Set addedFiles, + final Set changedFiles, final Predicate fileFilter) { + final String canonicalWorkingDir = FilesUtils.toCanonical(workingDir).getAbsolutePath() + File.separator; + final Predicate isInWorkingDirSubDir = file -> FilesUtils.toCanonical(newFile(workTreeDir, file)) + .getAbsolutePath().startsWith(canonicalWorkingDir); + + final Set filteredAddedFiles = addedFiles.stream().filter(isInWorkingDirSubDir.and(fileFilter)) + .collect(Collectors.toSet()); + final Set filteredChangedFiles = changedFiles.stream().filter(isInWorkingDirSubDir.and(fileFilter)) + .collect(Collectors.toSet()); + + return new RepositoryStatus(oldCommitHash, newCommitHash, filteredAddedFiles, filteredChangedFiles, + GitUtils.getCommitedContents(workTreeDir, oldCommitHash, filteredChangedFiles), + getNewContents(workTreeDir, newCommitHash, filteredAddedFiles, filteredChangedFiles)); + } + + private static Map getNewContents(final File workTreeDir, final Optional newCommitHash, + final Set addedFiles, final Set changedFiles) { + if (!newCommitHash.isPresent()) { + return Collections.unmodifiableMap(Stream.concat(addedFiles.stream(), changedFiles.stream()) + .map(file -> new AbstractMap.SimpleImmutableEntry<>(file, newFile(workTreeDir, file))) + .collect(Collectors.toMap(Map.Entry::getKey, entry -> FilesUtils.readUtf8(entry.getValue())))); + } else { + return GitUtils.getCommitedContents(workTreeDir, newCommitHash.get(), Collections.unmodifiableSet( + Stream.concat(addedFiles.stream(), changedFiles.stream()).collect(Collectors.toSet()))); + } + } + + // Extra method to allow a SpotBugs exclusion of PATH_TRAVERSAL_IN. + private static File newFile(final File parent, final String child) { + return new File(parent, child); + } +} diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/git/GitHelper.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/git/GitUtils.java similarity index 83% rename from test-gap-analysis/src/main/java/com/scheible/testgapanalysis/git/GitHelper.java rename to test-gap-analysis/src/main/java/com/scheible/testgapanalysis/git/GitUtils.java index 1f3d24a..0d87115 100644 --- a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/git/GitHelper.java +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/git/GitUtils.java @@ -11,7 +11,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.TimeUnit; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; @@ -19,7 +18,6 @@ import org.eclipse.jgit.lib.ObjectLoader; import org.eclipse.jgit.lib.ObjectReader; import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.lib.internal.WorkQueue; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevTree; import org.eclipse.jgit.revwalk.RevWalk; @@ -27,16 +25,15 @@ import org.eclipse.jgit.treewalk.AbstractTreeIterator; import org.eclipse.jgit.treewalk.CanonicalTreeParser; import org.eclipse.jgit.treewalk.TreeWalk; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * * @author sj */ -public class GitHelper { +public abstract class GitUtils { - private static final Logger logger = LoggerFactory.getLogger(GitHelper.class); + private GitUtils() { + } static Repository open(final File currentDir) throws IOException { return new FileRepositoryBuilder().findGitDir(currentDir.getAbsoluteFile()).setMustExist(true).build(); @@ -46,7 +43,7 @@ static Map getCommitedContents(final File currentDir, final Stri final Set files) { final Map fileLastCommitContentMapping = new HashMap<>(); - try (Repository repository = GitHelper.open(currentDir)) { + try (Repository repository = GitUtils.open(currentDir)) { try (Git git = new Git(repository)) { try (RevWalk walk = new RevWalk(repository)) { @@ -95,7 +92,7 @@ static AbstractTreeIterator prepareTreeParser(final Repository repository, final } static List getCommitHashes(final File currentDir, final int count) { - try (Repository repository = GitHelper.open(currentDir)) { + try (Repository repository = GitUtils.open(currentDir)) { try (Git git = new Git(repository)) { final Iterator logs = git.log().call().iterator(); final List result = new ArrayList<>(count); @@ -117,15 +114,4 @@ static List getCommitHashes(final File currentDir, final int count) { throw new UncheckedIOException(ex); } } - - public static void shutdownWorkerQueue() { - WorkQueue.getExecutor().shutdown(); - try { - while (!WorkQueue.getExecutor().awaitTermination(500, TimeUnit.MILLISECONDS)) { - logger.info("Awaiting completion of git worker threads."); - } - } catch (InterruptedException ex) { - throw new IllegalStateException(ex); - } - } } diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/git/RepositoryStatus.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/git/RepositoryStatus.java index 91831b9..7a17feb 100644 --- a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/git/RepositoryStatus.java +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/git/RepositoryStatus.java @@ -1,29 +1,9 @@ package com.scheible.testgapanalysis.git; -import java.io.File; -import java.io.IOException; -import java.io.UncheckedIOException; -import java.util.AbstractMap.SimpleImmutableEntry; import java.util.Collections; -import java.util.HashSet; -import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Optional; import java.util.Set; -import java.util.function.Predicate; -import java.util.stream.Collectors; -import java.util.stream.Stream; - -import org.eclipse.jgit.api.Git; -import org.eclipse.jgit.api.Status; -import org.eclipse.jgit.api.errors.GitAPIException; -import org.eclipse.jgit.diff.DiffEntry; -import org.eclipse.jgit.lib.Constants; -import org.eclipse.jgit.lib.Ref; -import org.eclipse.jgit.lib.Repository; - -import com.scheible.testgapanalysis.common.Files2; /** * @@ -31,139 +11,26 @@ */ public class RepositoryStatus { - private final File workTreeDir; - private final String oldCommitHash; private final Optional newCommitHash; private final Set addedFiles; private final Set changedFiles; - private RepositoryStatus(final File workTreeDir, final String oldCommitHash, final Optional newCommitHash, - final Set addedFiles, final Set changedFiles) { - this.workTreeDir = workTreeDir; + private final Map oldContents; + private final Map newContents; + RepositoryStatus(final String oldCommitHash, final Optional newCommitHash, final Set addedFiles, + final Set changedFiles, final Map oldContents, + final Map newContents) { this.oldCommitHash = oldCommitHash; this.newCommitHash = newCommitHash; this.addedFiles = Collections.unmodifiableSet(addedFiles); this.changedFiles = Collections.unmodifiableSet(changedFiles); - } - - public static RepositoryStatus ofWorkingCopyChanges(final File workingDir) { - final Set addedFiles = new HashSet<>(); - final Set changedFiles = new HashSet<>(); - - final Ref head; - File workTreeDir; - - try { - try (Repository repository = GitHelper.open(workingDir)) { - workTreeDir = repository.getWorkTree(); - try (Git git = new Git(repository)) { - head = repository.exactRef(Constants.HEAD); - - final Status status = git.status().call(); - - addedFiles.addAll(status.getAdded()); - addedFiles.addAll(status.getUntracked()); - - changedFiles.addAll(status.getModified()); - changedFiles.removeAll(status.getAdded()); - } catch (GitAPIException ex2) { - throw new IllegalStateException(ex2); - } - } - } catch (IOException ex) { - throw new UncheckedIOException(ex); - } - - return filterFiles(workingDir, workTreeDir, head.getObjectId().getName(), Optional.empty(), addedFiles, - changedFiles); - } - - public static RepositoryStatus ofCommitComparedToHead(final File workingDir, final String commitHash) { - return ofCommitsCompared(workingDir, commitHash, Constants.HEAD); - } - - public static RepositoryStatus ofCommitsCompared(final File workingDir, final String oldObjectId, - final String newObjectId) { - final Set addedFiles = new HashSet<>(); - final Set changedFiles = new HashSet<>(); - - String resolvedNewObjectId; - File workTreeDir; - - try { - try (Repository repository = GitHelper.open(workingDir)) { - workTreeDir = repository.getWorkTree(); - try (Git git = new Git(repository)) { - resolvedNewObjectId = Constants.HEAD.equals(newObjectId) - ? repository.exactRef(Constants.HEAD).getObjectId().getName() - : newObjectId; - - final List diff = git.diff() - .setOldTree(GitHelper.prepareTreeParser(repository, oldObjectId)) - .setNewTree(GitHelper.prepareTreeParser(repository, resolvedNewObjectId)).call(); - - for (final DiffEntry entry : diff) { - if (entry.getChangeType() == DiffEntry.ChangeType.ADD - || entry.getChangeType() == DiffEntry.ChangeType.COPY) { - addedFiles.add(entry.getNewPath()); - - } else if (entry.getChangeType() == DiffEntry.ChangeType.MODIFY - || entry.getChangeType() == DiffEntry.ChangeType.RENAME) { - changedFiles.add(entry.getNewPath()); - } - } - } catch (GitAPIException ex2) { - throw new IllegalStateException(ex2); - } - } - } catch (IOException ex) { - throw new UncheckedIOException(ex); - } - - return filterFiles(workingDir, workTreeDir, oldObjectId, Optional.of(resolvedNewObjectId), addedFiles, - changedFiles); - } - - /** - * Only include files that are in the working dir or in a sub-directory of working dir. - */ - private static RepositoryStatus filterFiles(final File workingDir, final File workTreeDir, - final String oldCommitHash, final Optional newCommitHash, final Set addedFiles, - final Set changedFiles) { - final String canonicalWorkingDir = Files2.toCanonical(workingDir).getAbsolutePath() + File.separator; - final Predicate isInWorkingDirSubDir = file -> Files2.toCanonical(appendChildFile(workTreeDir, file)) - .getAbsolutePath().startsWith(canonicalWorkingDir); - - return new RepositoryStatus(workTreeDir, oldCommitHash, newCommitHash, - addedFiles.stream().filter(isInWorkingDirSubDir).collect(Collectors.toSet()), - changedFiles.stream().filter(isInWorkingDirSubDir).collect(Collectors.toSet())); - } - - private static File appendChildFile(final File parent, final String child) { - return new File(parent, child); - } - - public Map getOldContents(final Predicate fileFilter) { - return GitHelper.getCommitedContents(workTreeDir, oldCommitHash, - changedFiles.stream().filter(fileFilter).collect(Collectors.toSet())); - } - - public Map getNewContents(final Predicate fileFilter) { - if (!newCommitHash.isPresent()) { - return Collections - .unmodifiableMap(Stream.concat(addedFiles.stream(), changedFiles.stream()).filter(fileFilter) - .map(file -> new SimpleImmutableEntry<>(file, appendChildFile(workTreeDir, file))) - .collect(Collectors.toMap(Entry::getKey, entry -> Files2.readUtf8(entry.getValue())))); - } else { - return GitHelper.getCommitedContents(workTreeDir, newCommitHash.get(), - Collections.unmodifiableSet(Stream.concat(addedFiles.stream(), changedFiles.stream()) - .filter(fileFilter).collect(Collectors.toSet()))); - } + this.oldContents = Collections.unmodifiableMap(oldContents); + this.newContents = Collections.unmodifiableMap(newContents); } public String getOldCommitHash() { @@ -181,4 +48,12 @@ public Set getAddedFiles() { public Set getChangedFiles() { return changedFiles; } + + public Map getOldContents() { + return oldContents; + } + + public Map getNewContents() { + return newContents; + } } diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/jacoco/JaCoCoHelper.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/jacoco/JaCoCoReportParser.java similarity index 90% rename from test-gap-analysis/src/main/java/com/scheible/testgapanalysis/jacoco/JaCoCoHelper.java rename to test-gap-analysis/src/main/java/com/scheible/testgapanalysis/jacoco/JaCoCoReportParser.java index 2cf86f4..e392b6d 100644 --- a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/jacoco/JaCoCoHelper.java +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/jacoco/JaCoCoReportParser.java @@ -36,13 +36,13 @@ * * @author sj */ -public class JaCoCoHelper { +public class JaCoCoReportParser { - public static Set getMethodCoverage(final String reportXmlContent) { + public Set getMethodCoverage(final String reportXmlContent) { return getMethodCoverage(new InputSource(new StringReader(reportXmlContent))); } - public static Set getMethodCoverage(final File reportXmlFile) { + public Set getMethodCoverage(final File reportXmlFile) { try (InputStream input = Files.newInputStream(reportXmlFile.toPath())) { return getMethodCoverage(new InputSource(input)); } catch (final IOException ex) { @@ -50,7 +50,7 @@ public static Set getMethodCoverage(final File reportXml } } - private static Set getMethodCoverage(final InputSource inputSource) { + private Set getMethodCoverage(final InputSource inputSource) { try { final DocumentBuilderFactory builderFactory = DocumentBuilderFactory.newInstance(); builderFactory.setFeature(FEATURE_SECURE_PROCESSING, true); @@ -93,11 +93,11 @@ private static Set getMethodCoverage(final InputSource i * report for unit and integration tests). This methods takes care of merging multiple entries for the same * method. */ - public static Set getMethodCoverage(final Set reportFiles) { + public Set getMethodCoverage(final Set reportFiles) { final Map> methods = new HashMap<>(); for (final File reportFile : reportFiles) { - for (final MethodWithCoverageInfo method : JaCoCoHelper.getMethodCoverage(reportFile)) { + for (final MethodWithCoverageInfo method : getMethodCoverage(reportFile)) { final String key = method.getClassName() + method.getName() + method.getDescription() + method.getLine(); methods.computeIfAbsent(key, k -> new HashSet<>()).add(method); diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/jacoco/MethodWithCoverageInfo.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/jacoco/MethodWithCoverageInfo.java index 2607dca..8d5688a 100644 --- a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/jacoco/MethodWithCoverageInfo.java +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/jacoco/MethodWithCoverageInfo.java @@ -4,7 +4,7 @@ import java.util.Objects; import java.util.Optional; -import com.scheible.testgapanalysis.common.JavaMethodUtil; +import com.scheible.testgapanalysis.common.JavaMethodUtils; /** * @@ -23,8 +23,8 @@ public class MethodWithCoverageInfo { public MethodWithCoverageInfo(final String className, final String name, final String description, final int line, final int coveredInstructionCount) { this.className = className; - this.simpleClassName = JavaMethodUtil.getSimpleName(className, "/"); - this.enclosingSimpleName = JavaMethodUtil.getSimpleName(simpleClassName, "$"); + this.simpleClassName = JavaMethodUtils.getSimpleName(className, "/"); + this.enclosingSimpleName = JavaMethodUtils.getSimpleName(simpleClassName, "$"); this.name = name; this.description = description; this.line = line; diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/jacoco/resolver/CoverageResolver.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/jacoco/resolver/CoverageResolver.java index ae27094..095beca 100644 --- a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/jacoco/resolver/CoverageResolver.java +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/jacoco/resolver/CoverageResolver.java @@ -3,8 +3,8 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; -import static com.scheible.testgapanalysis.common.JavaMethodUtil.normalizeMethodArguments; -import static com.scheible.testgapanalysis.common.JavaMethodUtil.parseDescriptorArguments; +import static com.scheible.testgapanalysis.common.JavaMethodUtils.normalizeMethodArguments; +import static com.scheible.testgapanalysis.common.JavaMethodUtils.parseDescriptorArguments; import java.util.ArrayList; import java.util.Comparator; diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/JavaParserHelper.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/JavaParser.java similarity index 91% rename from test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/JavaParserHelper.java rename to test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/JavaParser.java index 77bf8a5..f12e9a9 100644 --- a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/JavaParserHelper.java +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/JavaParser.java @@ -10,7 +10,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; -import com.github.javaparser.JavaParser; import com.github.javaparser.ParseProblemException; import com.github.javaparser.ParseResult; import com.github.javaparser.ParserConfiguration; @@ -35,16 +34,16 @@ * * @author sj */ -public class JavaParserHelper { +public class JavaParser { - public static Set getMethods(final String code) { + public Set getMethods(final String code) { final Set result = new HashSet<>(); final AtomicBoolean debugMode = new AtomicBoolean(false); final ParserConfiguration configuration = new ParserConfiguration(); configuration.setLanguageLevel(LanguageLevel.BLEEDING_EDGE); - final JavaParser javaParser = new JavaParser(configuration); + final com.github.javaparser.JavaParser javaParser = new com.github.javaparser.JavaParser(configuration); final ParseResult parserResult = javaParser.parse(code); if (!parserResult.isSuccessful()) { @@ -66,7 +65,7 @@ public void visit(final ClassOrInterfaceDeclaration node, final Void arg) { public void visit(final ConstructorDeclaration node, final Void arg) { if (node.getRange().isPresent()) { final Range range = node.getRange().get(); - final String relevantCode = Masker.apply(code, range, findMasks(node), debugMode.get()); + final String relevantCode = MaskUtils.apply(code, range, findMasks(node), debugMode.get()); final List argumentTypes = node.getParameters().stream().map(Parameter::getType) .map(t -> t.asString() + (((Parameter) t.getParentNode().get()).isVarArgs() ? "[]" : "")) .collect(Collectors.toList()); @@ -95,7 +94,7 @@ public void visit(final ConstructorDeclaration node, final Void arg) { public void visit(final InitializerDeclaration node, final Void arg) { if (node.getRange().isPresent()) { final Range range = node.getRange().get(); - final String relevantCode = Masker.apply(code, range, findMasks(node), debugMode.get()); + final String relevantCode = MaskUtils.apply(code, range, findMasks(node), debugMode.get()); result.add(new ParsedMethod( node.isStatic() ? MethodType.STATIC_INITIALIZER : MethodType.INITIALIZER, @@ -111,7 +110,7 @@ public void visit(final InitializerDeclaration node, final Void arg) { public void visit(final MethodDeclaration node, final Void arg) { if (node.getRange().isPresent() && node.getBody().isPresent()) { final Range range = node.getRange().get(); - final String relevantCode = Masker.apply(code, range, findMasks(node), debugMode.get()); + final String relevantCode = MaskUtils.apply(code, range, findMasks(node), debugMode.get()); result.add(new ParsedMethod(node.isStatic() ? MethodType.STATIC_METHOD : MethodType.METHOD, ParserUtils.getTopLevelFqn(node), ParserUtils.getScope(node), node.getNameAsString(), @@ -126,7 +125,7 @@ public void visit(final MethodDeclaration node, final Void arg) { public void visit(final LambdaExpr node, final Void arg) { if (node.getRange().isPresent()) { final Range range = node.getRange().get(); - final String relevantCode = Masker.apply(code, range, findMasks(node), debugMode.get()); + final String relevantCode = MaskUtils.apply(code, range, findMasks(node), debugMode.get()); result.add(new ParsedMethod(MethodType.LAMBDA_METHOD, ParserUtils.getTopLevelFqn(node), ParserUtils.getScope(node), "lambda", relevantCode, ParserUtils.getCodeLines(node), diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/Masker.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/MaskUtils.java similarity index 96% rename from test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/Masker.java rename to test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/MaskUtils.java index 7c3a8bc..764b378 100644 --- a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/Masker.java +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/MaskUtils.java @@ -9,7 +9,10 @@ * * @author sj */ -public class Masker { +public abstract class MaskUtils { + + private MaskUtils() { + } /** * Replaces all masked areas of the code in the passed range with a whitespace. diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/ParsedMethod.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/ParsedMethod.java index 50ecf43..148ef2c 100644 --- a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/ParsedMethod.java +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/ParsedMethod.java @@ -11,7 +11,7 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; -import com.scheible.testgapanalysis.common.JavaMethodUtil; +import com.scheible.testgapanalysis.common.JavaMethodUtils; /** * @@ -52,7 +52,7 @@ public ParsedMethod(final MethodType methodType, final String topLevelTypeFqn, f final Optional outerDeclaringType) { this.methodType = methodType; this.topLevelTypeFqn = topLevelTypeFqn; - this.topLevelSimpleName = JavaMethodUtil.getSimpleName(topLevelTypeFqn, "."); + this.topLevelSimpleName = JavaMethodUtils.getSimpleName(topLevelTypeFqn, "."); this.scope = unmodifiableList(scope); this.enclosingSimpleName = scope.isEmpty() ? topLevelSimpleName : scope.get(scope.size() - 1); this.name = name; diff --git a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/ParserUtils.java b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/ParserUtils.java index 83dd358..2e47694 100644 --- a/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/ParserUtils.java +++ b/test-gap-analysis/src/main/java/com/scheible/testgapanalysis/parser/ParserUtils.java @@ -24,7 +24,10 @@ * * @author sj */ -public class ParserUtils { +public abstract class ParserUtils { + + private ParserUtils() { + } /** * Checks for any child nodes that are not comments. diff --git a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/DebugCoverageResolutionTest.java b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/DebugCoverageResolutionTest.java index e4a1bb6..829566a 100644 --- a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/DebugCoverageResolutionTest.java +++ b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/DebugCoverageResolutionTest.java @@ -6,7 +6,8 @@ import org.junit.Test; -import com.scheible.testgapanalysis.jacoco.JaCoCoHelper; +import com.scheible.testgapanalysis.jacoco.JaCoCoReportParser; +import com.scheible.testgapanalysis.parser.JavaParser; /** * @@ -18,7 +19,14 @@ public class DebugCoverageResolutionTest { public void testRun() { // There are no (real) JaCoCo reports available at this time --> use the testing ones and just make sure that // it reads the methods correctly. - assertThat(DebugCoverageResolution.run(new File("."), new File("./src/main/java"), - JaCoCoHelper.findJaCoCoReportFiles(new File("./target/test-classes")))).isNotNull(); + final DebugCoverageResolution debugCoverageResolution = new DebugCoverageResolution(new JavaParser(), + new JaCoCoReportParser()); + DebugCoverageResolutionReport xxx; + assertThat(xxx = debugCoverageResolution.run(new File("D:\\third-party\\commons-collections"), + new File("D:\\third-party\\commons-collections\\src\\main\\java"), + JaCoCoReportParser + .findJaCoCoReportFiles(new File("D:\\third-party\\commons-collections\\target\\site")))) + .isNotNull(); + "".trim(); } } diff --git a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/TestGapAnalysisTest.java b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/TestGapAnalysisTest.java index 63357b7..d2fc06f 100644 --- a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/TestGapAnalysisTest.java +++ b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/TestGapAnalysisTest.java @@ -7,7 +7,10 @@ import org.junit.Test; -import com.scheible.testgapanalysis.jacoco.JaCoCoHelper; +import com.scheible.testgapanalysis.analysis.Analysis; +import com.scheible.testgapanalysis.git.GitDiffer; +import com.scheible.testgapanalysis.jacoco.JaCoCoReportParser; +import com.scheible.testgapanalysis.parser.JavaParser; /** * @@ -19,8 +22,9 @@ public class TestGapAnalysisTest { public void testRunTestGapAnalysisWithReferenceCommit() { // There are no (real) JaCoCo reports available at this time --> use the testing ones and just make sure that // it reads the methods correctly. - final TestGapReport report = TestGapAnalysis.run(new File("."), - JaCoCoHelper.findJaCoCoReportFiles(new File("./target/test-classes")), + final TestGapAnalysis testGapAnalysis = createTestGapAnalysis(); + final TestGapReport report = testGapAnalysis.run(new File("."), + JaCoCoReportParser.findJaCoCoReportFiles(new File("./target/test-classes")), Optional.of("756a25318e23bebace82f8317f3a57e43204901a")); assertThat(report).isNotNull(); } @@ -29,8 +33,13 @@ public void testRunTestGapAnalysisWithReferenceCommit() { public void testRunTestGapAnalysisWithWorkingCopyComparison() { // There are no (real) JaCoCo reports available at this time --> use the testing ones and just make sure that // it reads the methods correctly. - final TestGapReport report = TestGapAnalysis.run(new File("."), - JaCoCoHelper.findJaCoCoReportFiles(new File("./target/test-classes")), Optional.empty()); + final TestGapAnalysis testGapAnalysis = createTestGapAnalysis(); + final TestGapReport report = testGapAnalysis.run(new File("."), + JaCoCoReportParser.findJaCoCoReportFiles(new File("./target/test-classes")), Optional.empty()); assertThat(report).isNotNull(); } + + private static TestGapAnalysis createTestGapAnalysis() { + return new TestGapAnalysis(new Analysis(new JavaParser()), new JaCoCoReportParser(), new GitDiffer()); + } } diff --git a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/analysis/AnalysisTest.java b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/analysis/AnalysisTest.java index 8c526ae..6b64f3c 100644 --- a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/analysis/AnalysisTest.java +++ b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/analysis/AnalysisTest.java @@ -1,7 +1,6 @@ package com.scheible.testgapanalysis.analysis; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -16,9 +15,9 @@ import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; -import com.scheible.testgapanalysis.TestGapAnalysis; import com.scheible.testgapanalysis.git.RepositoryStatus; import com.scheible.testgapanalysis.jacoco.MethodWithCoverageInfo; +import com.scheible.testgapanalysis.parser.JavaParser; /** * @@ -38,7 +37,7 @@ public void testSampleScenario() { fileContentMapping.put("Changed.java", "package test; public class Changed { private void doAction() { \"\".trim(); }}"); return fileContentMapping; - }).when(repositoryStatus).getOldContents(any()); + }).when(repositoryStatus).getOldContents(); doAnswer((Answer>) (InvocationOnMock iom) -> { final Map fileContentMapping = new HashMap<>(); @@ -46,14 +45,14 @@ public void testSampleScenario() { fileContentMapping.put("Changed.java", "package test; public class Changed { private void doAction() { \"\".size(); }}"); return fileContentMapping; - }).when(repositoryStatus).getNewContents(any()); + }).when(repositoryStatus).getNewContents(); final Set coverageInfo = new HashSet<>( Arrays.asList(new MethodWithCoverageInfo("test.Added", "doIt", "", 1, 1), new MethodWithCoverageInfo("test.Changed", "doAction", "", 1, 0))); - final AnalysisResult result = Analysis.perform(repositoryStatus, TestGapAnalysis.NON_TEST_JAVA_FILE, - coverageInfo); + final Analysis analysis = new Analysis(new JavaParser()); + final AnalysisResult result = analysis.perform(repositoryStatus, coverageInfo); assertThat( result.getUncoveredMethods().keySet().stream().map(pm -> pm.getTopLevelTypeFqn() + "#" + pm.getName())) .containsOnly("test.Changed#doAction"); diff --git a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/common/Files2Test.java b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/common/FileUtilsTest.java similarity index 78% rename from test-gap-analysis/src/test/java/com/scheible/testgapanalysis/common/Files2Test.java rename to test-gap-analysis/src/test/java/com/scheible/testgapanalysis/common/FileUtilsTest.java index c2ebc35..7015a80 100644 --- a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/common/Files2Test.java +++ b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/common/FileUtilsTest.java @@ -12,11 +12,11 @@ * * @author sj */ -public class Files2Test { +public class FileUtilsTest { @Test public void testToRelative() { - assertThat(Files2.toRelative(Paths.get("test", "it").toFile(), + assertThat(FilesUtils.toRelative(Paths.get("test", "it").toFile(), Sets.newHashSet(Paths.get("test", "it", "foo.txt").toFile()))).containsOnly("foo.txt"); } } diff --git a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/common/JavaMethodUtilTest.java b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/common/JavaMethodUtilTest.java index 1973090..424d7a6 100644 --- a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/common/JavaMethodUtilTest.java +++ b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/common/JavaMethodUtilTest.java @@ -2,10 +2,10 @@ import static java.util.Collections.emptyMap; -import static com.scheible.testgapanalysis.common.JavaMethodUtil.getNextClassPart; -import static com.scheible.testgapanalysis.common.JavaMethodUtil.getNextPrimitivePart; -import static com.scheible.testgapanalysis.common.JavaMethodUtil.normalizeMethodArguments; -import static com.scheible.testgapanalysis.common.JavaMethodUtil.parseDescriptorArguments; +import static com.scheible.testgapanalysis.common.JavaMethodUtils.getNextClassPart; +import static com.scheible.testgapanalysis.common.JavaMethodUtils.getNextPrimitivePart; +import static com.scheible.testgapanalysis.common.JavaMethodUtils.normalizeMethodArguments; +import static com.scheible.testgapanalysis.common.JavaMethodUtils.parseDescriptorArguments; import static org.assertj.core.api.Assertions.assertThat; import java.util.Arrays; diff --git a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/git/GitHelperTest.java b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/git/GitDifferTest.java similarity index 55% rename from test-gap-analysis/src/test/java/com/scheible/testgapanalysis/git/GitHelperTest.java rename to test-gap-analysis/src/test/java/com/scheible/testgapanalysis/git/GitDifferTest.java index 7ab78c4..0fa9be7 100644 --- a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/git/GitHelperTest.java +++ b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/git/GitDifferTest.java @@ -1,6 +1,6 @@ package com.scheible.testgapanalysis.git; -import static com.scheible.testgapanalysis.common.Files2.getWorkingDir; +import static com.scheible.testgapanalysis.common.FilesUtils.getWorkingDir; import static org.assertj.core.api.Assertions.assertThat; import java.util.List; @@ -13,23 +13,25 @@ * * @author sj */ -public class GitHelperTest { +public class GitDifferTest { + + private final GitDiffer gitDiffer = new GitDiffer(); @Test public void testRepositoryStatusAndContent() { - final RepositoryStatus status = RepositoryStatus.ofWorkingCopyChanges(getWorkingDir()); + final RepositoryStatus status = gitDiffer.ofWorkingCopyChanges(getWorkingDir(), file -> true); assertThat(status).isNotNull(); - final Map content = status.getOldContents((file) -> true); + final Map content = status.getOldContents(); assertThat(content).isNotNull(); } @Test public void testCompareToHead() { - final List commitHashes = GitHelper.getCommitHashes(getWorkingDir(), 5); + final List commitHashes = GitUtils.getCommitHashes(getWorkingDir(), 5); final String commitHash = commitHashes.get(commitHashes.size() - 1); - final RepositoryStatus status = RepositoryStatus.ofCommitComparedToHead(getWorkingDir(), commitHash); + final RepositoryStatus status = gitDiffer.ofCommitComparedToHead(getWorkingDir(), commitHash, file -> true); assertThat(status.getAddedFiles()).isNotNull(); assertThat(status.getChangedFiles()).isNotNull(); @@ -37,12 +39,12 @@ public void testCompareToHead() { @Test public void testCompareToCommits() { - final List commitHashes = GitHelper.getCommitHashes(getWorkingDir(), 5); + final List commitHashes = GitUtils.getCommitHashes(getWorkingDir(), 5); final String oldCommitHash = commitHashes.get(commitHashes.size() - 1); final String newCommitHash = commitHashes.get(0); - final RepositoryStatus status = RepositoryStatus.ofCommitsCompared(getWorkingDir(), oldCommitHash, - newCommitHash); + final RepositoryStatus status = GitDiffer.ofCommitsCompared(getWorkingDir(), oldCommitHash, newCommitHash, + file -> true); assertThat(status.getAddedFiles()).isNotNull(); assertThat(status.getChangedFiles()).isNotNull(); diff --git a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/jacoco/JaCoCoHelperTest.java b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/jacoco/JaCoCoReportParserTest.java similarity index 88% rename from test-gap-analysis/src/test/java/com/scheible/testgapanalysis/jacoco/JaCoCoHelperTest.java rename to test-gap-analysis/src/test/java/com/scheible/testgapanalysis/jacoco/JaCoCoReportParserTest.java index ccc831c..f4fd67b 100644 --- a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/jacoco/JaCoCoHelperTest.java +++ b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/jacoco/JaCoCoReportParserTest.java @@ -1,7 +1,6 @@ package com.scheible.testgapanalysis.jacoco; -import static com.scheible.testgapanalysis.jacoco.JaCoCoHelper.getIsNotChildOfSubDirsPredicate; -import static com.scheible.testgapanalysis.jacoco.JaCoCoHelper.getMethodCoverage; +import static com.scheible.testgapanalysis.jacoco.JaCoCoReportParser.getIsNotChildOfSubDirsPredicate; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.util.Sets.newLinkedHashSet; @@ -13,7 +12,7 @@ * * @author sj */ -public class JaCoCoHelperTest { +public class JaCoCoReportParserTest { private static final String REPORT = "\n" // + "\n" // @@ -38,7 +37,8 @@ public class JaCoCoHelperTest { @Test public void testMethodCoverage() { - assertThat(getMethodCoverage(REPORT)).containsOnly( // + final JaCoCoReportParser jaCoCoReportParser = new JaCoCoReportParser(); + assertThat(jaCoCoReportParser.getMethodCoverage(REPORT)).containsOnly( // new MethodWithCoverageInfo("com/scheible/testgapanalysis/git/GitHelper", "", "()V", 37, 0), new MethodWithCoverageInfo("com/scheible/testgapanalysis/git/GitHelper", "open", "(Ljava/io/File;)Lorg/eclipse/jgit/lib/Repository;", 42, 12)); diff --git a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/jacoco/resolver/AbstractIntegrationTest.java b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/jacoco/resolver/AbstractIntegrationTest.java index e624222..19d8ff8 100644 --- a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/jacoco/resolver/AbstractIntegrationTest.java +++ b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/jacoco/resolver/AbstractIntegrationTest.java @@ -32,7 +32,7 @@ import org.jacoco.report.IReportVisitor; import org.jacoco.report.xml.XMLFormatter; -import com.scheible.testgapanalysis.jacoco.JaCoCoHelper; +import com.scheible.testgapanalysis.jacoco.JaCoCoReportParser; import com.scheible.testgapanalysis.jacoco.MethodWithCoverageInfo; import com.scheible.testgapanalysis.parser.ParsedMethod; import com.scheible.testgapanalysis.parser.ParsedMethod.MethodType; @@ -205,7 +205,8 @@ public Class loadClass(final String name, final boolean resolve) throws Class final Set parsedMethods = parseJavaTestSource(testClass, filterTypes); final String xmlOutput = getCoverageReportXml(sessionInfos, executionData, coverageBuilder); - final Set methodCoverage = JaCoCoHelper.getMethodCoverage(xmlOutput); + final JaCoCoReportParser jaCoCoReportParser = new JaCoCoReportParser(); + final Set methodCoverage = jaCoCoReportParser.getMethodCoverage(xmlOutput); return new CoverageResolution(parsedMethods, methodCoverage, CoverageResolver.with(methodCoverage).resolve(parsedMethods)); diff --git a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/JavaParserHelperTest.java b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/JavaParserTest.java similarity index 96% rename from test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/JavaParserHelperTest.java rename to test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/JavaParserTest.java index 9ffd972..55ce034 100644 --- a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/JavaParserHelperTest.java +++ b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/JavaParserTest.java @@ -24,7 +24,7 @@ * * @author sj */ -public class JavaParserHelperTest { +public class JavaParserTest { public static class MethodParsing { @@ -112,7 +112,7 @@ public void testInnerClassConstructor() throws IOException { assertThat(parseMethods(InnerClassConstructor.class, INNER_CLASS_CONSTRUCTOR)) .containsOnly(new AssertableMethod(INNER_CLASS_CONSTRUCTOR, "")) // .first() - .matches(am -> am.getParsedMethod().getOuterDeclaringType().equals(Optional.of("JavaParserHelperTest")), + .matches(am -> am.getParsedMethod().getOuterDeclaringType().equals(Optional.of("JavaParserTest")), "has outer declaring type"); } @@ -133,7 +133,7 @@ public void testLambdaParsing() throws IOException { @Test public void testRecordParsing() { // records require Java 16, test-gap source code is Java 8 --> parse from string - assertThat(JavaParserHelper.getMethods("class Foo {\n" + // + assertThat(new JavaParser().getMethods("class Foo {\n" + // " record TestRecord(String value) { \n" + // " TestRecord(T val) {\n" + // " this(null);\n" + // @@ -166,7 +166,7 @@ public void testMethodMasking() throws IOException { private Stream parseMethods(final Class clazz, final MethodType... filterTypes) throws IOException { - return parseJavaTestSource(clazz, filterTypes).stream().map(JavaParserHelperTest::toAssertableMethod); + return parseJavaTestSource(clazz, filterTypes).stream().map(JavaParserTest::toAssertableMethod); } private static AssertableMethod toAssertableMethod(final ParsedMethod parsedMethod) { diff --git a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/MaskerTest.java b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/MaskUtilsTest.java similarity index 79% rename from test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/MaskerTest.java rename to test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/MaskUtilsTest.java index 14532b3..04fe692 100644 --- a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/MaskerTest.java +++ b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/MaskUtilsTest.java @@ -15,7 +15,7 @@ * * @author sj */ -public class MaskerTest { +public class MaskUtilsTest { private static final String TEST_CLASS = "" // + "class TestClass {\n" + " \n" // @@ -30,19 +30,20 @@ public class MaskerTest { @Test public void testNoMasks() { - assertThat(Masker.apply(TEST_CLASS, TEST_METHOD_RANGE, emptyList())).startsWith("void test()").endsWith("}\n"); + assertThat(MaskUtils.apply(TEST_CLASS, TEST_METHOD_RANGE, emptyList())).startsWith("void test()") + .endsWith("}\n"); } @Test public void testSingleMask() { - assertThat(Masker.apply(TEST_CLASS, TEST_METHOD_RANGE, + assertThat(MaskUtils.apply(TEST_CLASS, TEST_METHOD_RANGE, Arrays.asList(new Range(new Position(3, 8), new Position(3, 11))), true)).startsWith("void ####()") .endsWith("}\n"); } @Test public void testTwoOverlappingMasks() { - assertThat(Masker.apply(TEST_CLASS, TEST_METHOD_RANGE, + assertThat(MaskUtils.apply(TEST_CLASS, TEST_METHOD_RANGE, Arrays.asList(new Range(new Position(3, 8), new Position(3, 10)), new Range(new Position(3, 9), new Position(3, 11))), true)).startsWith("void ####()").endsWith("}\n"); @@ -50,7 +51,7 @@ public void testTwoOverlappingMasks() { @Test public void testMultilineMask() { - assertThat(Masker.apply(TEST_CLASS, TEST_METHOD_RANGE, + assertThat(MaskUtils.apply(TEST_CLASS, TEST_METHOD_RANGE, Arrays.asList(new Range(new Position(3, 8), new Position(4, 5))), true)) .startsWith("void ########\n" + "#####/ comment").endsWith("}\n"); } diff --git a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/TestClassSourceJavaParser.java b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/TestClassSourceJavaParser.java index 69e338f..1b5ba22 100644 --- a/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/TestClassSourceJavaParser.java +++ b/test-gap-analysis/src/test/java/com/scheible/testgapanalysis/parser/TestClassSourceJavaParser.java @@ -44,7 +44,8 @@ public static Set parseJavaTestSource(final Class testClass, fi final Set filterTypesSet = new HashSet<>(Arrays.asList(filterTypes)); final ClassWithSource classWithSource = readJavaTestSource(testClass); - final Set parsedMethods = JavaParserHelper.getMethods(classWithSource.source).stream() + final JavaParser javaParser = new JavaParser(); + final Set parsedMethods = javaParser.getMethods(classWithSource.source).stream() .filter(m -> (!m.getScope().isEmpty() && m.getScope().get(0).equals(testClass.getSimpleName())) || classWithSource.testClass.equals(classWithSource.topLevelClass)) .filter(m -> filterTypesSet.contains(m.getMethodType())).collect(Collectors.toSet());