=== mup_ is now known as mup === StoneTable is now known as aisrael === bodie__ is now known as bodie_ === bodie_ is now known as Guest39866 === jcsackett_ is now known as jcsackett === meetingology` is now known as meetingology === ev_ is now known as ev === Guest39866 is now known as bodie_ [15:33] hello, could I get another review for https://github.com/juju/testing/pull/29 ? [17:15] 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] 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] but can those really be considered the same *path* [17:17] it's possible that there is a problem with my concept of "path" [17:18] Umm, it uses os.samefile to check [17:18] So it would actually work [17:18] It's kind of a workaround for symlinks [17:18] Because they don't work on windows [17:18] And yeah it doesn't matter if there's a file there [17:18] It's multipurpose [17:19] If the path doesn't match it checks if it points to the same file [17:20] 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] i.e., maybe C:/PROGRA~1 matches both C:/Program Files and C:/Programming [17:22] It wouldn't [17:22] 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] my thought is only that perhaps if you're comparing paths, you don't want the actual files to have to exist [17:23] but, it's just a thought :) [17:24] they don't have to [17:24] if they don't exist it exits [17:25] if you follow the logic it first looks at the strings themselves [17:25] and if they don't match then it looks if both of them point to actual files [17:25] I'm specifically looking at your comment on line 205 [17:25] and only after that if checks whether the files are actually the same file [17:26] thus, the files don't have to exist _unless_ x [17:26] but, I'm saying C:/Programming could a match for C:/PROGRA~1 even if neither file exists [17:27] or PROGRA~2 [17:27] there's really no way of knowing [17:27] since it's many-to-many name mapping [17:27] I just don't know if the user will expect the files to have to exist in that case [17:27] they would not match because if they don't exist it would throw that specific error [17:28] 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] I don't understand exactly what you're getting at here [17:28] think of specific use cases [17:29] well what you're testing is whether the files match in those cases, not whether the paths match [17:29] and if the files don't exist, and the user is trying to check something about paths, maybe that could be problematic [17:29] 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] as I said -- just a thought :) [17:30] yeah I got that it's just a thought but I wanted to clarify it for you [17:31] 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] hmm, okay [17:32] it's probably only going to get used inside windows tests, but it's also *nix compatible [17:44] thanks for the review, i'll fix that in a bit [17:49] actually I had a test for that already [17:49] commented on github