[15:33] <bogdanteleaga> hello, could I get another review for https://github.com/juju/testing/pull/29 ?
[17:15] <bodie_> bogdanteleaga, the PathChecker is only for checking that the paths are the same, right?  it shouldn't matter whether there is a file there
[17:16] <bodie_> I guess the problem is that "C:/Program Files" and "C:/Programming" have a name collision and could be C:/PROGRA~1 and C:/PROGRA~2
[17:16] <bodie_> but can those really be considered the same *path*
[17:17] <bodie_> it's possible that there is a problem with my concept of "path"
[17:18] <bogdanteleaga> Umm, it uses os.samefile to check
[17:18] <bogdanteleaga> So it would actually work
[17:18] <bogdanteleaga> It's kind of a workaround for symlinks
[17:18] <bogdanteleaga> Because they don't work on windows
[17:18] <bogdanteleaga> And yeah it doesn't matter if there's a file there
[17:18] <bogdanteleaga> It's multipurpose
[17:19] <bogdanteleaga> If the path doesn't match it checks if it points to the same file
[17:20] <bodie_> no, I know it would work in that case, the question is more whether the files should have to exist for the path to check correctly
[17:21] <bodie_> i.e., maybe C:/PROGRA~1 matches both C:/Program Files and C:/Programming
[17:22] <bogdanteleaga> It wouldn't
[17:22] <bodie_> well it's not a one-to-one mapping, no, rather a many-to-many, so the concept of comparing those paths is intrinsically problematic unless you compare the underlying files
[17:23] <bodie_> my thought is only that perhaps if you're comparing paths, you don't want the actual files to have to exist
[17:23] <bodie_> but, it's just a thought :)
[17:24] <bogdanteleaga> they don't have to
[17:24] <bogdanteleaga> if they don't exist it exits
[17:25] <bogdanteleaga> if you follow the logic it first looks at the strings themselves
[17:25] <bogdanteleaga> and if they don't match then it looks if both of them point to actual files
[17:25] <bodie_> I'm specifically looking at your comment on line 205
[17:25] <bogdanteleaga> and only after that if checks whether the files are actually the same file
[17:26] <bodie_> thus, the files don't have to exist _unless_ x
[17:26] <bodie_> but, I'm saying C:/Programming could a match for C:/PROGRA~1 even if neither file exists
[17:27] <bodie_> or PROGRA~2
[17:27] <bodie_> there's really no way of knowing
[17:27] <bodie_> since it's many-to-many name mapping
[17:27] <bodie_> I just don't know if the user will expect the files to have to exist in that case
[17:27] <bogdanteleaga> they would not match because if they don't exist it would throw that specific error
[17:28] <bogdanteleaga> the only case in which you would expect 2 paths to be the same if they contain different strings is the case in which they point to the same file
[17:28] <bogdanteleaga> I don't understand exactly what you're getting at here
[17:28] <bogdanteleaga> think of specific use cases
[17:29] <bodie_> well what you're testing is whether the files match in those cases, not whether the paths match
[17:29] <bodie_> and if the files don't exist, and the user is trying to check something about paths, maybe that could be problematic
[17:29] <bodie_> however, I'm not truly certain about your intent, so I thought I would just mention it here instead of holding back your review
[17:30] <bodie_> as I said -- just a thought :)
[17:30] <bogdanteleaga> yeah I got that it's just a thought but I wanted to clarify it for you
[17:31] <bogdanteleaga> the issue is that if the paths are not the same string-wise the only other possible case is a symlink or hardlink
[17:31] <bodie_> hmm, okay
[17:32] <bogdanteleaga> it's probably only going to get used inside windows tests, but it's also *nix compatible
[17:44] <bogdanteleaga> thanks for the review, i'll fix that in a bit
[17:49] <bogdanteleaga> actually I had a test for that already
[17:49] <bogdanteleaga> commented on github