[PATCH] pathutil: add doctests for canonpath()

[Matt Harbison]

On Sat, 04 Nov 2017 22:11:21 -0400, Yuya Nishihara <yuya at tcha.org> wrote:

> On Sat, 4 Nov 2017 09:16:03 -0400, Matt Harbison wrote:
>> > On Nov 4, 2017, at 4:03 AM, Yuya Nishihara <yuya at tcha.org> wrote:
>> >> On Fri, 03 Nov 2017 23:26:40 -0400, Matt Harbison wrote:
>> >> # HG changeset patch
>> >> # User Matt Harbison <matt_harbison at yahoo.com>
>> >> # Date 1509762170 14400
>> >> #      Fri Nov 03 22:22:50 2017 -0400
>> >> # Node ID 7d9abeb82f021b4b96162a719f4a44a0175c0828
>> >> # Parent  3649c3f2cd90c8aec395ca8af5adae33defff12c
>> >> pathutil: add doctests for canonpath()
>> >>
>> >> This is a followup to f445b10dc7fb.  Since there's no way to ensure  
>> that more
>> >> drive letters than C: exist, this seems like the only way to test  
>> it.  This is
>> >> enough to catch the f445b10dc7fb scenario, as well as CWD outside of  
>> the repo
>> >> when the path isn't prefixed with path/to/repo.
>> >
>> > Maybe we can add an hghave rule to check if d: drive exists, is  
>> different from
>> > the TESTTMP, and can chdir into it?
>>
>> I thought about that, but this allows everyone running tests on Windows  
>> to be able to see the error if it regresses, regardless of their  
>> setup.  That seems important, given how few people run Windows tests.   
>> I’m not sure what the Windows test system is currently configured to  
>> do, for instance (it’s been a WIP because of some unexpected errors  
>> running under buildbot, and I don’t think it is totally squared away  
>> yet).
>
> Okay, that makes some sense. I'll queue this patch, thanks.
>
> My concern is that the doctest is too limited in scope to cover possible  
> bugs
> caused by Windows drive letter. Maybe we could add a safer alternative to
> os.path.relpath() and ban the use by check-code.

Seems like a good idea.  There are a few more uses of it, and I'm pretty  
sure that at least cmdutil._conflictsmsg() is broke too, based on passing  
repo.root and cwd.  (I guess thg doesn't use this function?)

I haven't been able to create the issue with the origpath code, and like I  
said in the 'share --relative' patch, the error is useful there.  Is this  
one of those things we can ban, but just record the few places it's needed  
in test-check-code.t?