-
Notifications
You must be signed in to change notification settings - Fork 6.1k
8361587: AssertionError in File.listFiles() when path is empty and -esa is enabled #26224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
👋 Welcome back bpb! A progress list of the required criteria for merging this PR into |
@bplb This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 69 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
Webrevs
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The implementation change looks good, just a few minor comments on the test additions.
/integrate |
Going to push as commit eefbfdc.
Your commit was automatically rebased without conflicts. |
/backport :jdk25 |
@bplb the backport was successfully created on the branch backport-bplb-eefbfdce-jdk25 in my personal fork of openjdk/jdk. To create a pull request with this backport targeting openjdk/jdk:jdk25, just click the following link: The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:
If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk:
|
Hello Brian, I'm late to this discussion. I think there are a couple of more places in this code which need attention. The private I updated the diff --git a/test/jdk/java/io/File/EmptyPath.java b/test/jdk/java/io/File/EmptyPath.java
index d0c9beddc08..b747c61f5d1 100644
--- a/test/jdk/java/io/File/EmptyPath.java
+++ b/test/jdk/java/io/File/EmptyPath.java
@@ -28,8 +28,10 @@
*/
import java.io.File;
+import java.io.FileFilter;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
+import java.io.FilenameFilter;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.FileStore;
@@ -242,6 +244,19 @@ public void listFiles() throws IOException {
assertEquals(nioSet, ioSet);
}
+
+ @Test
+ public void listFiles2() throws IOException {
+ final File f = new File("");
+ final File[] files = f.listFiles((FileFilter) null);
+ }
+
+ @Test
+ public void listFiles3() throws IOException {
+ final File f = new File("");
+ final File[] files = f.listFiles((FilenameFilter) null);
+ }
+
These 2 new test methods continue to fail against mainline master which has the changes integrated from this PR:
|
@bplb Can you check the test coverage again? Its important that EmptyTest exercises every method. |
Yes. |
Thanks for catching this, @jaikiran. As you no doubt already observed, this has been fixed by #26353 in the mainline and backported by #26361 to JDK 25 before the rampdown phase 2 deadline. |
Thank you Brian for addressing these issues very quickly. |
Changes to address
File.listFiles
invoked on an empty path. This fixes an oversight in #22821.Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26224/head:pull/26224
$ git checkout pull/26224
Update a local copy of the PR:
$ git checkout pull/26224
$ git pull https://git.openjdk.org/jdk.git pull/26224/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 26224
View PR using the GUI difftool:
$ git pr show -t 26224
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26224.diff
Using Webrev
Link to Webrev Comment