Skip to content

8360411: [TEST] open/test/jdk/java/io/File/MaxPathLength.java Refactor extract method to encapsulate Windows specific test logic #26193

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions test/jdk/java/io/File/MaxPathLength.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2002, 2013, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2002, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -60,14 +60,7 @@ public static void main(String[] args) throws Exception {

// test long paths on windows
// And these long pathes cannot be handled on Solaris and Mac platforms
if (isWindows) {
String name = fileName;
while (name.length() < MAX_LENGTH) {
testLongPath (20, name, false);
testLongPath (20, name, true);
name = getNextName(name);
}
}
testLongPathOnWindows();
}

private static String getNextName(String fName) {
Expand Down Expand Up @@ -199,4 +192,15 @@ static void testLongPath(int max, String fn,
}
}
}

private static void testLongPathOnWindows () throws Exception {
if (isWindows) {
String name = fileName;
while (name.length() < MAX_LENGTH) {
testLongPath (20, name, false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: do you think there is a need for a space before the (? Looks odd imo. But as the whole file uses this, I'm fine with you leaving this as is

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, was just being coherent

testLongPath (20, name, true);
name = getNextName(name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name doesn't seem to be used after. Do you think it might be beneficial to add an assert here to check if the result is the same as expected ?
Same applies to the main method

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it would be beneficial.

Would this be a valid assert to add after those while loops?

assert name.length() >= MAX_LENGTH : "Unexpected final name length: " + name.length();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a second look, I can see that the name is used in the while loops. My bad, sorry.
The test is increasing the name length in getNextName, so it increases in each iteration. The test logic itself is in testLongPath and checks if the name is valid. So, in my opinion, checking the value of the name after the while loop probably will not be beneficial

}
}
}
}