Here's a better idea for how to write the unit test.
First, we put something in the SymbolicLinkFileFilter class like this:
public class SymbolicLinkFileFilter {
private static LinkTester linkTester = new LinkTester(); // defined below
static setLinkTesterForUnitTests(LinkTester unitTestLinkTester) { //
package access!
linkTester = unitTestLinkTester;
}
// rest of class code here
// everywhere the class needs to test if a file is a Symbolic link
calls this method:
private static class LinkTester {
public boolean isSymbolicLink(File file) { ... }
}
}
Now we can write a unit test that replaces the LinkTester with a custom
tester that overrides the behavior on Windows only. This way, our unit test
just verifies the logic of the SymbolicLinkFileFilter class. We're still
stuck with the problem of how to test the default LinkTester on Windows,
but we've made the problem a lot smaller.
— Miguel
On Sun, Apr 16, 2023 at 1:10 PM Miguel Muñoz <swingguy1...@gmail.com> wrote:
> "The class per its javadoc filters for symbolic links on files."
>
> It sounds like it's only supposed to pass files that are symbolic links.
> If that's the idea, the class doesn't work at all. If not, we need a
> clearer description.
>
> As for unit tests, maybe the test should detect the Windows OS and just
> return. To the Javadocs, we would add a note-to-developers explaining that
> unit tests can't work on Windows, and describing how to test the class
> separately. The alternative is a lot more complicated: On windows, without
> admin rights, it could return an Error with a message saying that it needs
> Admin Rights to run properly. And maybe we could allow the user to create a
> symbolic link somewhere, and define an environment variable with the path
> to that file which lets the test run properly. (I know that's very kludgy,
> but this situation can't be solved normally.) I can't write this test
> because I don't have a Windows machine.
>
> — Miguel
>
> On Fri, Apr 14, 2023 at 7:10 AM Gary Gregory <garydgreg...@gmail.com>
> wrote:
>
>> Hi Miguel,
>>
>> Thank you for reviewing. A better unit test would help of course ;-) but
>> testing links can be a pain as the JVM requires admin rights on Windows 10
>> at least. This might not be an issue for GitHub but be aware of it if you
>> run builds locally.
>>
>> The class per its javadoc filters for symbolic links on files.
>>
>> Gary
>>
>> On Fri, Apr 14, 2023, 10:05 Gary Gregory <garydgreg...@gmail.com> wrote:
>>
>> > Let's assume this is for IO, not VFS; in the future, please prefix email
>> > subjects with
>> > [IO] or [VFS] or whatever Commons component is being discussed.
>> >
>> > TY!
>> > Gary
>> >
>> > On Thu, Apr 13, 2023, 01:59 Miguel Muñoz <swingguy1...@gmail.com>
>> wrote:
>> >
>> >> It's not clear what exactly this filter is supposed to do. From its
>> >> behavior, I gather that it's supposed to distinguish files from
>> >> directories, treating Symbolic links as ordinary files. There are two
>> >> reasons why this isn't clear.
>> >>
>> >> 1. This behavior (usually) seems identical to that of a filter that
>> just
>> >> relies on File.isDirectory().
>> >> 2. The current behavior is inconsistent:
>> >>
>> >> I made a test directory with four entries: a file, a directory, and a
>> >> symbolic link to each.
>> >>
>> >> I wrote code using SymbolicLinkFileFilter.INSTANCE as a FileFilter, a
>> >> FileNameFilter, and a PathFilter. I created a second filter that
>> relied on
>> >> File.isDirectory(). The two filters behaved the same for three of the
>> >> files, but differed for the symbolic link to the directory. (I'm not
>> sure
>> >> if the filter should treat that as a file or a directory, since it's
>> not a
>> >> directory, but it resolves as one.)
>> >>
>> >> Used as a FileFilter and a FileNameFilter, the accept() methods
>> returned
>> >> false. Used as a PathFilter, it returned FileVisitResult.CONTINUE,
>> which
>> >> essentially means true.
>> >>
>> >> Looking through the code, the difference in behavior is apparently
>> because
>> >> the call to resolve the PathFilter is the only one that calls
>> >> Files.isSymbolicLink().
>> >>
>> >> I could fix this, but I don't know which behavior is correct.
>> >>
>> >> I'll send you my test code if you want it.
>> >>
>> >> — Miguel Muñoz
>> >>
>> >
>>
>
