From cb0cd4a40b2692d7862c26040a2f0ff82337aa3d Mon Sep 17 00:00:00 2001 From: Henry Coles Date: Thu, 6 Oct 2022 18:10:07 +0100 Subject: [PATCH 1/6] search for source outside of package dirs --- .../tooling/DirectorySourceLocator.java | 98 +++++++-------- .../tooling/SmartSourceLocator.java | 8 +- .../tooling/DirectorySourceLocatorTest.java | 117 ++++++++++++++---- 3 files changed, 141 insertions(+), 82 deletions(-) diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java b/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java index e110ff4a1..38925d239 100644 --- a/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java @@ -16,7 +16,6 @@ import java.io.BufferedInputStream; import java.io.File; -import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStreamReader; import java.io.Reader; @@ -25,74 +24,67 @@ import java.nio.file.Path; import java.util.Collection; import java.util.Optional; -import java.util.function.Function; -import java.util.stream.Stream; import org.pitest.classinfo.ClassName; -import org.pitest.functional.Streams; import org.pitest.mutationtest.SourceLocator; import org.pitest.util.Unchecked; public class DirectorySourceLocator implements SourceLocator { - private final Path root; - private final Function> fileToReader; - - private static final class FileToReader implements Function> { - + private final Path root; private final Charset inputCharset; - private FileToReader(Charset inputCharset) { - this.inputCharset = inputCharset; + public DirectorySourceLocator(Path root, Charset inputCharset) { + this.root = root; + this.inputCharset = inputCharset; } @Override - public Optional apply(final Path f) { - if (Files.exists(f)) { - try { - return Optional.of(new InputStreamReader(new BufferedInputStream(Files.newInputStream(f)), - inputCharset)); - } catch (final FileNotFoundException e) { - return Optional.empty(); - } catch (IOException ex) { - throw Unchecked.translateCheckedException(ex); - } - } - return Optional.empty(); - } - - } + public Optional locate(Collection classes, String fileName) { - DirectorySourceLocator(Path root, Function> fileToReader) { - this.root = root; - this.fileToReader = fileToReader; - } + if (!Files.exists(root)) { + return Optional.empty(); + } - public DirectorySourceLocator(Path root, Charset inputCharset) { - this(root, new FileToReader(inputCharset)); - } + // look for matching filename in directories matching its package. + Optional path = classes.stream() + .map(ClassName::fromString) + .map(ClassName::getPackage) + .distinct() + .map(c -> c.asJavaName().replace(".", File.separator) + File.separator + fileName) + .map(file -> root.resolve(file)) + .filter(Files::exists) + .filter(Files::isRegularFile) + .findFirst(); - @Override - public Optional locate(Collection classes, String fileName) { - final Stream matches = classes.stream().flatMap(classNameToSourceFileReader(fileName)); - return matches.findFirst(); - } + // If there is no file in the expected location (kotlin file?), search from the root, but + // in this case we cannot know if we have the right file if the same name occurs more than once in the file tree + // (cannot do this as an or as only introduced in java 9) + if (path.isPresent()) { + return path + .map(this::toReader); + } else { + return searchFromRoot(fileName) + .map(this::toReader); + } + } - private Function> classNameToSourceFileReader( - final String fileName) { - return className -> { - if (className.contains(".")) { - ClassName classPackage = ClassName.fromString(className).getPackage(); - String path = classPackage.asJavaName().replace(".", File.separator); - return locate(path + File.separator + fileName); - } else { - return locate(fileName); - } - }; - } + private Reader toReader(Path path) { + try { + return new InputStreamReader(new BufferedInputStream(Files.newInputStream(path)), + inputCharset); + } catch (IOException e) { + throw Unchecked.translateCheckedException(e); + } + } - private Stream locate(final String fileName) { - return Streams.fromOptional(this.fileToReader.apply(root.resolve(fileName))); - } + private Optional searchFromRoot(String fileName) { + try { + return Files.find(root, 100, (path, attributes) -> path.getFileName().toString().equals(fileName)) + .findFirst(); + } catch (IOException e) { + throw Unchecked.translateCheckedException(e); + } + } } diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/SmartSourceLocator.java b/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/SmartSourceLocator.java index ff2bbf1ce..811d84b06 100644 --- a/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/SmartSourceLocator.java +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/SmartSourceLocator.java @@ -20,6 +20,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Collection; +import java.util.Collections; import java.util.function.Function; import org.pitest.functional.FCollection; @@ -50,6 +51,10 @@ private Function> collectChildren(final int depth) { private Collection collectDirectories(Path root, int depth) { try { + if (!Files.exists(root)) { + return Collections.emptyList(); + } + return Files.find(root, depth, (unused, attributes) -> attributes.isDirectory()) .collect(Collectors.toList()); @@ -60,8 +65,7 @@ private Collection collectDirectories(Path root, int depth) { } @Override - public Optional locate(final Collection classes, - final String fileName) { + public Optional locate(Collection classes, String fileName) { for (final SourceLocator each : this.children) { final Optional reader = each.locate(classes, fileName); if (reader.isPresent()) { diff --git a/pitest-entry/src/test/java/org/pitest/mutationtest/tooling/DirectorySourceLocatorTest.java b/pitest-entry/src/test/java/org/pitest/mutationtest/tooling/DirectorySourceLocatorTest.java index 6368cf2da..b3b3f1755 100644 --- a/pitest-entry/src/test/java/org/pitest/mutationtest/tooling/DirectorySourceLocatorTest.java +++ b/pitest-entry/src/test/java/org/pitest/mutationtest/tooling/DirectorySourceLocatorTest.java @@ -14,52 +14,115 @@ */ package org.pitest.mutationtest.tooling; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static java.util.Collections.singletonList; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; -import java.io.File; +import java.io.IOException; import java.io.Reader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.Collections; -import java.util.function.Function; +import java.util.Optional; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; -import org.mockito.Mock; -import org.mockito.MockitoAnnotations; -import java.util.Optional; +import org.junit.rules.TemporaryFolder; public class DirectorySourceLocatorTest { - private DirectorySourceLocator testee; - private Path root; + @Rule + public TemporaryFolder folder = new TemporaryFolder(); + Path root; - @Mock - Function> locator; + private DirectorySourceLocator testee; @Before public void setUp() { - MockitoAnnotations.openMocks(this); - this.root = Paths.get("."); - this.testee = new DirectorySourceLocator(this.root, this.locator); - when(this.locator.apply(any(Path.class))) - .thenReturn(Optional. empty()); + root = folder.getRoot().toPath(); + this.testee = new DirectorySourceLocator(this.root, StandardCharsets.UTF_8); + } + + @Test + public void locatesSourceForClassesInDefaultPackage() throws Exception { + createFile(root.resolve("Foo.java"), "foo"); + createFile(root.resolve("Bar.java"), "bar"); + Optional actual = testee.locate(singletonList("Foo"), "Foo.java"); + assertThat(actual).isPresent(); + assertThat(content(actual)).isEqualTo("foo"); + } + + @Test + public void locatesSourceForClassesInNamedPackages() throws Exception { + createFile(root.resolve("com/example/Foo.java"), "foo"); + createFile(root.resolve("com/example/Bar.java"), "bar"); + Optional actual = testee.locate(singletonList("com.example.Foo"), "Foo.java"); + assertThat(content(actual)).isEqualTo("foo"); + } + + @Test + public void findsFileInPackageBeforeOneAtRoot() throws Exception { + createFile(root.resolve("com/example/Foo.java"), "this one"); + createFile(root.resolve("Foo.java"), "not this one"); + Optional actual = testee.locate(singletonList("com.example.Foo"), "Foo.java"); + assertThat(content(actual)).isEqualTo("this one"); } @Test - public void shouldLocateSourceForClassesInDefaultPackage() { - this.testee.locate(Collections.singletonList("Foo"), "Foo.java"); - Path expected = root.resolve("Foo.java"); - verify(this.locator).apply(expected); + public void findsFileInRootBeforeOneInWrongPackage() throws Exception { + createFile(root.resolve("com/example/other/Foo.java"), "not this one"); + createFile(root.resolve("Foo.java"), "this one"); + Optional actual = testee.locate(singletonList("com.example.Foo"), "Foo.java"); + assertThat(content(actual)).isEqualTo("this one"); } @Test - public void shouldLocateSourceForClassesInNamedPackages() { - this.testee - .locate(Collections.singletonList("com.example.Foo"), "Foo.java"); - Path expected = root.resolve("com").resolve("example").resolve("Foo.java"); - verify(this.locator).apply(expected); + public void usesFileInRightPAckage() throws Exception { + createFile(root.resolve("com/example/other/Foo.java"), "not this one"); + createFile(root.resolve("com/example/correct/Foo.java"), "this one"); + Optional actual = testee.locate(singletonList("com.example.correct.Foo"), "Foo.java"); + assertThat(content(actual)).isEqualTo("this one"); + } + + @Test + public void doesNotTryToReadDirectories() throws Exception { + Files.createDirectories(root.resolve("com/example/Foo.java")); + createFile(root.resolve("Foo.java"), "this one"); + Optional actual = testee.locate(singletonList("com.example.Foo"), "Foo.java"); + assertThat(content(actual)).isEqualTo("this one"); + } + + @Test + public void doesNotErrorWhenRootDoesNotExist() { + testee = new DirectorySourceLocator(this.root.resolve("doesNotExist"), StandardCharsets.UTF_8); + assertThatCode(() -> testee.locate(singletonList("com"), "Bar.java")) + .doesNotThrowAnyException(); + } + + private void createFile(Path file, String content) throws IOException { + if (file.getParent() != null) { + Files.createDirectories(file.getParent()); + } + + Files.write(file, content.getBytes(StandardCharsets.UTF_8)); + } + + private String content(Optional reader) throws Exception { + if (reader.isPresent()) { + return content(reader.get()); + } + return ""; + } + + private String content(Reader reader) throws Exception { + String s = ""; + int ch; + while ((ch = reader.read()) != -1) { + s += (char) ch; + } + reader.close(); + return s; } } From fa1d58d5bd0f85a3ec4a0cbb93a9b82e670e9fa6 Mon Sep 17 00:00:00 2001 From: Henry Coles Date: Fri, 7 Oct 2022 08:29:28 +0100 Subject: [PATCH 2/6] ensure directories are not read --- .../pitest/mutationtest/tooling/DirectorySourceLocator.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java b/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java index 38925d239..cd5030a84 100644 --- a/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java @@ -80,7 +80,8 @@ private Reader toReader(Path path) { private Optional searchFromRoot(String fileName) { try { - return Files.find(root, 100, (path, attributes) -> path.getFileName().toString().equals(fileName)) + return Files.find(root, 100, + (path, attributes) -> path.getFileName().toString().equals(fileName) && attributes.isRegularFile()) .findFirst(); } catch (IOException e) { throw Unchecked.translateCheckedException(e); From 7f80959d757145340eec275cbe989d52c554c549 Mon Sep 17 00:00:00 2001 From: Henry Coles Date: Fri, 7 Oct 2022 09:20:03 +0100 Subject: [PATCH 3/6] close resource allocating streams --- .../mutationtest/tooling/DirectorySourceLocator.java | 9 ++++++--- .../pitest/mutationtest/tooling/SmartSourceLocator.java | 6 ++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java b/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java index cd5030a84..8963cf55b 100644 --- a/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java @@ -24,6 +24,7 @@ import java.nio.file.Path; import java.util.Collection; import java.util.Optional; +import java.util.stream.Stream; import org.pitest.classinfo.ClassName; import org.pitest.mutationtest.SourceLocator; @@ -80,9 +81,11 @@ private Reader toReader(Path path) { private Optional searchFromRoot(String fileName) { try { - return Files.find(root, 100, - (path, attributes) -> path.getFileName().toString().equals(fileName) && attributes.isRegularFile()) - .findFirst(); + try (Stream matches = Files.find(root, 100, + (path, attributes) -> path.getFileName().toString().equals(fileName) && attributes.isRegularFile())) { + return matches.findFirst(); + } + } catch (IOException e) { throw Unchecked.translateCheckedException(e); } diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/SmartSourceLocator.java b/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/SmartSourceLocator.java index 811d84b06..e6b4950ac 100644 --- a/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/SmartSourceLocator.java +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/SmartSourceLocator.java @@ -26,6 +26,7 @@ import org.pitest.functional.FCollection; import java.util.Optional; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.pitest.mutationtest.SourceLocator; import org.pitest.util.Unchecked; @@ -55,8 +56,9 @@ private Collection collectDirectories(Path root, int depth) { return Collections.emptyList(); } - return Files.find(root, depth, (unused, attributes) -> attributes.isDirectory()) - .collect(Collectors.toList()); + try (Stream matches = Files.find(root, depth, (unused, attributes) -> attributes.isDirectory())) { + return matches.collect(Collectors.toList()); + } } catch (IOException ex) { throw Unchecked.translateCheckedException(ex); From 11776f7e450232705f97c040a9d8e70def224f46 Mon Sep 17 00:00:00 2001 From: Henry Coles Date: Fri, 7 Oct 2022 09:36:02 +0100 Subject: [PATCH 4/6] check uses file from correct package --- .../tooling/DirectorySourceLocatorTest.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pitest-entry/src/test/java/org/pitest/mutationtest/tooling/DirectorySourceLocatorTest.java b/pitest-entry/src/test/java/org/pitest/mutationtest/tooling/DirectorySourceLocatorTest.java index b3b3f1755..bc44c4691 100644 --- a/pitest-entry/src/test/java/org/pitest/mutationtest/tooling/DirectorySourceLocatorTest.java +++ b/pitest-entry/src/test/java/org/pitest/mutationtest/tooling/DirectorySourceLocatorTest.java @@ -23,7 +23,6 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Collections; import java.util.Optional; import org.junit.Before; @@ -70,6 +69,17 @@ public void findsFileInPackageBeforeOneAtRoot() throws Exception { assertThat(content(actual)).isEqualTo("this one"); } + @Test + public void findsFileInCorrectPackageBeforeWronglyPackagedOnes() throws Exception { + createFile(root.resolve("com/example/correct/Foo.java"), "this one"); + createFile(root.resolve("Foo.java"), "not this one"); + createFile(root.resolve("com/example/Foo.java"), "not this one"); + createFile(root.resolve("com/example/wrong/Foo.java"), "not this one"); + createFile(root.resolve("com/example/correct/wrong/Foo.java"), "not this one"); + Optional actual = testee.locate(singletonList("com.example.correct.Foo"), "Foo.java"); + assertThat(content(actual)).isEqualTo("this one"); + } + @Test public void findsFileInRootBeforeOneInWrongPackage() throws Exception { createFile(root.resolve("com/example/other/Foo.java"), "not this one"); From 3f8c230fad2b151feb4e8231f3ba60e9e8497481 Mon Sep 17 00:00:00 2001 From: Henry Coles Date: Fri, 7 Oct 2022 09:52:50 +0100 Subject: [PATCH 5/6] disable test tied to os specific behaviour --- .../tooling/DirectorySourceLocator.java | 9 ++++- .../tooling/DirectorySourceLocatorTest.java | 35 ++++++++++++++----- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java b/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java index 8963cf55b..c2a588286 100644 --- a/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java @@ -52,7 +52,7 @@ public Optional locate(Collection classes, String fileName) { .map(ClassName::fromString) .map(ClassName::getPackage) .distinct() - .map(c -> c.asJavaName().replace(".", File.separator) + File.separator + fileName) + .map(c -> toFileName(c, fileName)) .map(file -> root.resolve(file)) .filter(Files::exists) .filter(Files::isRegularFile) @@ -70,6 +70,13 @@ public Optional locate(Collection classes, String fileName) { } } + private String toFileName(ClassName packge, String fileName) { + if (packge.equals("")) { + return fileName; + } + return packge.asJavaName().replace(".", File.separator) + File.separator + fileName; + } + private Reader toReader(Path path) { try { return new InputStreamReader(new BufferedInputStream(Files.newInputStream(path)), diff --git a/pitest-entry/src/test/java/org/pitest/mutationtest/tooling/DirectorySourceLocatorTest.java b/pitest-entry/src/test/java/org/pitest/mutationtest/tooling/DirectorySourceLocatorTest.java index bc44c4691..c216f3a60 100644 --- a/pitest-entry/src/test/java/org/pitest/mutationtest/tooling/DirectorySourceLocatorTest.java +++ b/pitest-entry/src/test/java/org/pitest/mutationtest/tooling/DirectorySourceLocatorTest.java @@ -23,9 +23,11 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Collection; import java.util.Optional; import org.junit.Before; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -71,19 +73,26 @@ public void findsFileInPackageBeforeOneAtRoot() throws Exception { @Test public void findsFileInCorrectPackageBeforeWronglyPackagedOnes() throws Exception { - createFile(root.resolve("com/example/correct/Foo.java"), "this one"); - createFile(root.resolve("Foo.java"), "not this one"); - createFile(root.resolve("com/example/Foo.java"), "not this one"); + createFile(root.resolve("com/example/correct/Foo.java"), "correct"); + createFile(root.resolve("Foo.java"), "in package default"); + createFile(root.resolve("com/example/Foo.java"), "example"); createFile(root.resolve("com/example/wrong/Foo.java"), "not this one"); createFile(root.resolve("com/example/correct/wrong/Foo.java"), "not this one"); - Optional actual = testee.locate(singletonList("com.example.correct.Foo"), "Foo.java"); - assertThat(content(actual)).isEqualTo("this one"); + + assertThat(findFor("com.example.correct.Foo", "Foo.java")).isEqualTo("correct"); + assertThat(findFor("Foo", "Foo.java")).isEqualTo("in package default"); + assertThat(findFor("com.example.Foo", "Foo.java")).isEqualTo("example"); } @Test - public void findsFileInRootBeforeOneInWrongPackage() throws Exception { - createFile(root.resolve("com/example/other/Foo.java"), "not this one"); - createFile(root.resolve("Foo.java"), "this one"); + @Ignore + // Docs suggest that Files.walk/find should search depth first, but behaviour seems + // to be OS dependent in practice. Windows ci on azure looks to search depth first, linux + // and mac find the root file. Fortunately we don't actually care about the behaviour in this case + // either file might be the one the user intended + public void findsFileInWrongPackageBeforeRoot() throws Exception { + createFile(root.resolve("com/example/other/Foo.java"), "this one"); + createFile(root.resolve("Foo.java"), "not this one"); Optional actual = testee.locate(singletonList("com.example.Foo"), "Foo.java"); assertThat(content(actual)).isEqualTo("this one"); } @@ -111,6 +120,16 @@ public void doesNotErrorWhenRootDoesNotExist() { .doesNotThrowAnyException(); } + private String findFor(String clazz, String file) throws Exception { + return findFor(singletonList(clazz), file); + } + + private String findFor(Collection classes, String file) throws Exception { + Optional actual = testee.locate(classes, file); + return content(actual); + } + + private void createFile(Path file, String content) throws IOException { if (file.getParent() != null) { Files.createDirectories(file.getParent()); From 014d82dfdac11791bfb17052ce7d5136b294971b Mon Sep 17 00:00:00 2001 From: Henry Coles Date: Fri, 7 Oct 2022 14:24:35 +0100 Subject: [PATCH 6/6] support default package when matching source file to class --- .../org/pitest/mutationtest/tooling/DirectorySourceLocator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java b/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java index c2a588286..c523d2793 100644 --- a/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java +++ b/pitest-entry/src/main/java/org/pitest/mutationtest/tooling/DirectorySourceLocator.java @@ -71,7 +71,7 @@ public Optional locate(Collection classes, String fileName) { } private String toFileName(ClassName packge, String fileName) { - if (packge.equals("")) { + if (packge.asJavaName().equals("")) { return fileName; } return packge.asJavaName().replace(".", File.separator) + File.separator + fileName;