Skip to content
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

ignore missing files when deleting TempDirectory #140

Merged

Conversation

panchenko
Copy link
Contributor

I've got an exception when TempDirectory was deleting its contents because one of the files has been deleted at the same time by the process started from test.
I think such disappearing files should be ignored by TempDirectory.

java.nio.file.NoSuchFileException: /var/folders/5p/6vn1lg4d6x3b5rmct9nkn96w0000gn/T/junit1862350231264789347/logs/foo.pid
    at sun.nio.fs.UnixException.translateToIOException(UnixException.java:86)
    at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)
    at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)
    at sun.nio.fs.UnixFileAttributeViews$Basic.readAttributes(UnixFileAttributeViews.java:55)
    at sun.nio.fs.UnixFileSystemProvider.readAttributes(UnixFileSystemProvider.java:144)
    at java.nio.file.Files.readAttributes(Files.java:1737)
    at java.nio.file.FileTreeWalker.getAttributes(FileTreeWalker.java:219)
    at java.nio.file.FileTreeWalker.visit(FileTreeWalker.java:276)
    at java.nio.file.FileTreeWalker.next(FileTreeWalker.java:372)
    at java.nio.file.Files.walkFileTree(Files.java:2706)
    at java.nio.file.Files.walkFileTree(Files.java:2742)
    at org.junitpioneer.jupiter.TempDirectory$CloseablePath.deleteAllFilesAndDirectories(TempDirectory.java:275)
    at org.junitpioneer.jupiter.TempDirectory$CloseablePath.close(TempDirectory.java:267)
...

I hereby agree to the terms of the JUnit Pioneer Contributor License Agreement.

@sbrannen
Copy link

sbrannen commented Apr 2, 2019

Please note that the TempDirectory extension in JUnit Pioneer is effectively deprecated (see #136) and was moved to JUnit Jupiter in Jupiter 5.4.

Also, a similar bug was fixed in junit-team/junit5#1801.

Though, your proposed fixed adds a check on a per-file level. Perhaps you'd like to propose that particular fix for JUnit Jupiter?

@nipafx
Copy link
Member

nipafx commented Oct 22, 2019

Since Pioneer's extension is not yet deprecated (and #136 makes it look like it may survive for the time being) I'm going to rebase and merge this.

@nipafx nipafx force-pushed the ignore_missing_files_when_deleting branch from 5621956 to b7abcc3 Compare October 22, 2019 10:51
Comment on lines -289 to +313
Files.delete(path);
// without races by multiple threads, a call to `Files::delete` would suffice
// because the tree walker doesn't visit non-existing files; since race
// conditions can't be ruled out, `Files::deleteIfExists` is the safer approach
Files.deleteIfExists(path);
Copy link
Member

@nipafx nipafx Oct 22, 2019

Choose a reason for hiding this comment

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

I wondered about this change and tried to write a test for it, but couldn't. In the end, I came up with the explanation the comment describes. Would be nice of somebody could sanity-check it.

.toArray(DiscoverySelector[]::new);
//@formatter:on
LauncherDiscoveryRequest request = request().selectors(selectors).build();
return executeTests(request);
}

private MethodSelector selectMethodWithPossibleParameters(Class<?> type, String methodSignature) {
Copy link
Member

Choose a reason for hiding this comment

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

Could this be interesting for the Jupiter test superclass? It makes it easier to launch a bunch of tests methods if they have parameters. Nota bene: I also wondered whether Jupiter could be a little more lenient and select a test method by name only (i.e. ignoring parameter types) if there is only one method of that name.

@nipafx nipafx requested a review from sbrannen October 22, 2019 10:59
@nipafx nipafx force-pushed the ignore_missing_files_when_deleting branch from b7abcc3 to a59ca03 Compare November 30, 2019 21:08
@nipafx
Copy link
Member

nipafx commented Nov 30, 2019

Proposed commit message:

TempDirectory ignores missing files when cleaning up

If a process under test deletes temporary files *while* the extension
is cleaning up after itself, the extension may throw an exception
because it tries to delete a no longer existing file. This is
unneccessary and easily fixed by using `Files.deleteIfExists`.

PR: #140

@nipafx nipafx merged commit 633e334 into junit-pioneer:master Nov 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants