TemporaryFolder.newFolder(String...) supports path separator #1406
Conversation
kcooney
force-pushed
the
TemporaryFolder-support-path-separator
branch
from
6763c81
to
288c6df
Compare
| directory = new File(directory, "temp3"); | ||
| assertFileIsDirectory(directory); | ||
| directory = new File(directory, "temp4"); | ||
| assertFileIsDirectory(directory); |
There was a problem hiding this comment.
Wouldn't it be sufficient have a single assertion: assertFileIsDirectory(new File(tempFolder.getRoot(), "temp1/temp2/temp3/temp4")?
There was a problem hiding this comment.
I wasn't 100% certain that would work on all platforms
There was a problem hiding this comment.
I'm pretty sure it should. Can we assume it does until proven otherwise?
There was a problem hiding this comment.
Sure. I have no way to verify on Windows though.
| File file = root; | ||
| boolean lastMkdirsCallSuccessful = true; | ||
| for (int i = 0; i < paths.length; i++) { | ||
| relativePath = relativePath == null ? new File(paths[i]) : new File(relativePath, paths[i]); |
There was a problem hiding this comment.
According to the Javadoc of File this can be simplified to relativePath = new File(relativePath, paths[i]).
| if (new File(path).isAbsolute()) { | ||
| throw new IOException("folder path \'" + path + "\' is not a relative path"); | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we should also add a check for paths.length > 0. Because in that case we currently return getRoot() which isn't a "new fresh folder".
There was a problem hiding this comment.
Yes, I noticed that was the current behavior, too. I wasn't sure you wanted that in the same pull. I added the fix in a new commit. I also added a commit fixing some incorrect test method names.
kcooney
force-pushed
the
TemporaryFolder-support-path-separator
branch
from
288c6df
to
4a3cfd9
Compare
| @@ -63,7 +63,7 @@ public void newFileWithGivenFilenameThrowsIllegalArgumentExceptionIfFileExists() | |||
| } | |||
|
|
|||
| @Test(expected = IllegalStateException.class) | |||
| public void newFolderThrowsIllegalStateExceptionIfCreateWasNotInvoked() | |||
| public void newFolderThrowsIOExceptionIfCreateWasNotInvoked() | |||
There was a problem hiding this comment.
This one actually does throw an IllegalStateException.
kcooney
force-pushed
the
TemporaryFolder-support-path-separator
branch
from
4a3cfd9
to
109397b
Compare
The error messages on failures now include the full relative path that could not be created.
kcooney
force-pushed
the
TemporaryFolder-support-path-separator
branch
from
109397b
to
70a86fb
Compare
| /* | ||
| * Before checking if the paths are absolute paths, check if create() was ever called, | ||
| * and if it wasn't, throw IllegalStateException. | ||
| */ |
There was a problem hiding this comment.
I think the comment is wrong because I could not find the IllegalStateException.
There was a problem hiding this comment.
getRoot() throws IllegalStateException if before() wasn't (yet?) called.
|
That's a nice improvement. Thanks. |
No description provided.