From c3d041d9f36b0ae5c4c86a19587328a464cce5c3 Mon Sep 17 00:00:00 2001 From: Lukasz Slonina Date: Fri, 6 Jun 2025 10:10:28 +0200 Subject: [PATCH 1/5] Add withPosition instance method to allow reuse of resolved canonical file See #4630 --- .../engine/support/descriptor/FileSource.java | 20 +++++++++++++++++++ .../descriptor/FileSystemSourceTests.java | 16 +++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/FileSource.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/FileSource.java index 6e7a3ac95452..ff3eac4b08d2 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/FileSource.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/FileSource.java @@ -10,6 +10,7 @@ package org.junit.platform.engine.support.descriptor; +import static org.apiguardian.api.API.Status.EXPERIMENTAL; import static org.apiguardian.api.API.Status.STABLE; import java.io.File; @@ -77,6 +78,11 @@ private FileSource(File file, @Nullable FilePosition filePosition) { this.filePosition = filePosition; } + private FileSource(FileSource fileSource, FilePosition filePosition) { + this.file = fileSource.file; + this.filePosition = filePosition; + } + /** * Get the {@link URI} for the source {@linkplain #getFile file}. * @@ -104,6 +110,20 @@ public Optional getPosition() { return Optional.ofNullable(this.filePosition); } + /** + * Return a new {@code FileSource} based on this instance but with a different + * {@link FilePosition}. This avoids redundant canonical path resolution + * by reusing the already-canonical file. + * + * @param filePosition the new {@code FilePosition}; must not be {@code null} + * @return a new {@code FileSource} with the same file and updated position + */ + @API(status = EXPERIMENTAL, since = "6.0") + public FileSource withPosition(FilePosition filePosition) { + Preconditions.notNull(filePosition, "filePosition must not be null"); + return new FileSource(this, filePosition); + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/FileSystemSourceTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/FileSystemSourceTests.java index 802d2c47cda6..ddeb685efbb7 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/FileSystemSourceTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/FileSystemSourceTests.java @@ -76,6 +76,22 @@ void fileWithPosition() { assertThat(source.getPosition()).hasValue(position); } + @Test + void fileReuseWithPosition() { + var file = new File("test.txt"); + var position = FilePosition.from(42, 23); + var source = FileSource.from(file); + var sourceWithPosition = source.withPosition(position); + + assertThat(source.getUri()).isEqualTo(file.getAbsoluteFile().toURI()); + assertThat(source.getFile()).isEqualTo(file.getAbsoluteFile()); + assertThat(source.getPosition()).isEmpty(); + + assertThat(source).isNotSameAs(sourceWithPosition); + assertThat(source.getFile()).isSameAs(sourceWithPosition.getFile()); + assertThat(sourceWithPosition.getPosition()).hasValue(position); + } + @Test void equalsAndHashCodeForFileSource() { var file1 = new File("foo.txt"); From df8193c6584325e25956a363222d7fbfde3237b6 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Sat, 13 Sep 2025 11:22:30 +0200 Subject: [PATCH 2/5] Polish Javadoc and formatting and allow `null` --- .../engine/support/descriptor/FileSource.java | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/FileSource.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/FileSource.java index ff3eac4b08d2..b927defe1afe 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/FileSource.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/FileSource.java @@ -78,10 +78,10 @@ private FileSource(File file, @Nullable FilePosition filePosition) { this.filePosition = filePosition; } - private FileSource(FileSource fileSource, FilePosition filePosition) { + private FileSource(FileSource fileSource, @Nullable FilePosition filePosition) { this.file = fileSource.file; this.filePosition = filePosition; - } + } /** * Get the {@link URI} for the source {@linkplain #getFile file}. @@ -111,16 +111,18 @@ public Optional getPosition() { } /** - * Return a new {@code FileSource} based on this instance but with a different - * {@link FilePosition}. This avoids redundant canonical path resolution - * by reusing the already-canonical file. - * - * @param filePosition the new {@code FilePosition}; must not be {@code null} - * @return a new {@code FileSource} with the same file and updated position - */ + * {@return a new {@code FileSource} based on this instance but with the + * supplied {@link FilePosition}} + * + *

Calling this method rather than creating a new {@code FileSource} via + * {@link #from(File, FilePosition)} avoids the overhead of redundant + * canonical path resolution. + * + * @param filePosition the position in the source file; may be {@code null} + * @since 6.0 + */ @API(status = EXPERIMENTAL, since = "6.0") - public FileSource withPosition(FilePosition filePosition) { - Preconditions.notNull(filePosition, "filePosition must not be null"); + public FileSource withPosition(@Nullable FilePosition filePosition) { return new FileSource(this, filePosition); } From c13a9c8b6d22a709e043abcecbc6852860c71983 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Sat, 13 Sep 2025 11:27:20 +0200 Subject: [PATCH 3/5] Add to release notes --- .../docs/asciidoc/release-notes/release-notes-6.0.0-RC3.adoc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-RC3.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-RC3.adoc index fae30bcd5f32..75ce7de79cb4 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-RC3.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-6.0.0-RC3.adoc @@ -38,6 +38,9 @@ JUnit repository on GitHub. * New `Resource.from(String, URI)` static factory method for creating an `org.junit.platform.commons.support.Resource`. +* New `FileSource.withPosition(FilePosition)` method to avoid the overhead of redundant + canonicalization of files when using `FileSource.from(File, FilePosition)` with many + different `FilePosition` instances for the same `File`. [[release-notes-6.0.0-RC3-junit-jupiter]] From 0a42d35ea60cf9cf5aa2691f88feab8ae06e9da5 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Sat, 13 Sep 2025 11:38:15 +0200 Subject: [PATCH 4/5] Add link to `withPosition` --- .../org/junit/platform/engine/support/descriptor/FileSource.java | 1 + 1 file changed, 1 insertion(+) diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/FileSource.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/FileSource.java index b927defe1afe..ca8667ffad92 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/FileSource.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/FileSource.java @@ -54,6 +54,7 @@ public static FileSource from(File file) { * * @param file the source file; must not be {@code null} * @param filePosition the position in the source file; may be {@code null} + * @see #withPosition(FilePosition) */ public static FileSource from(File file, @Nullable FilePosition filePosition) { return new FileSource(file, filePosition); From 0ef4bf99943a04061256b796f2f03eb1da79af0b Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Sat, 13 Sep 2025 11:46:50 +0200 Subject: [PATCH 5/5] Return same instance when position is unchanged --- .../platform/engine/support/descriptor/FileSource.java | 10 +++++++++- .../support/descriptor/FileSystemSourceTests.java | 5 +++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/FileSource.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/FileSource.java index ca8667ffad92..ad869d878b5c 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/FileSource.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/FileSource.java @@ -112,9 +112,14 @@ public Optional getPosition() { } /** - * {@return a new {@code FileSource} based on this instance but with the + * {@return a {@code FileSource} based on this instance but with the * supplied {@link FilePosition}} * + *

If the supplied {@code FilePosition} + * {@linkplain Objects#equals(Object, Object) equals} the existing one, this + * method returns {@code this}. Otherwise, a new instance is created and + * returned. + * *

Calling this method rather than creating a new {@code FileSource} via * {@link #from(File, FilePosition)} avoids the overhead of redundant * canonical path resolution. @@ -124,6 +129,9 @@ public Optional getPosition() { */ @API(status = EXPERIMENTAL, since = "6.0") public FileSource withPosition(@Nullable FilePosition filePosition) { + if (Objects.equals(this.filePosition, filePosition)) { + return this; + } return new FileSource(this, filePosition); } diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/FileSystemSourceTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/FileSystemSourceTests.java index ddeb685efbb7..b05bae70d7b5 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/FileSystemSourceTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/FileSystemSourceTests.java @@ -90,6 +90,11 @@ void fileReuseWithPosition() { assertThat(source).isNotSameAs(sourceWithPosition); assertThat(source.getFile()).isSameAs(sourceWithPosition.getFile()); assertThat(sourceWithPosition.getPosition()).hasValue(position); + + assertThat(sourceWithPosition.withPosition(null).getPosition()).isEmpty(); + + assertThat(source.withPosition(null)).isSameAs(source); + assertThat(sourceWithPosition.withPosition(position)).isSameAs(sourceWithPosition); } @Test