Code review comment for lp:~luoyonggang/subvertpy/unittest-win32

2012/3/12 Jelmer Vernooij <email address hidden>

> > 2012/3/11 Jelmer Vernooij <email address hidden>
> >
> > > > Please read the test result carefully. And you will know why do the
> > > things in
> > > > this way.
> > > > The test result is under
> > > >
> > > > Description of the Change
> > > That only explains *what* is failing for you. It doesn't explain *why*
> it
> > > is failing, and why your fix addresses that. That's the bit I'm
> interested
> > > in.
> > It's failed just because Python's maketemp didn't return the corrent
> > tempfile file path.
> > For example, tempfile.mkdtemp() on windows will gives us with the file
> path
> > c:\users\uername\appdata\local\temp\hash_code , but the real file path
> > is C:\Users\username\AppData\Local\Temp\hash_code.
> > When I use
> > tempfile.mkdtemp(dir =
> > os.getenv("TEMP", None) or os.getenv("TMP", None))
> > it's will give us the exact file path with the windows file system.
> > TEMP and TMP is recorded by the OS. and even if TEMP and TMP is not
> exist,
> > it's sill works, just because dir accept None as
> > a default input.
> TEMP and TMP can be set by the user too as far as I know. What if the user
> has set them to "C:\users\USeRName\AppData\Local\Temp\hash_code" ? That's a
> perfectly valid path, as path lookups are case insensitive.
>
> If TEMP and TMP do not exist, we get the same behaviour as currently so
> users who have that will still hit that bg - and that means this fix is
> incomplete in that situation.
>
> > The unittest failed, is just because Subversion changed the file path
> > from c:\users\uername\appdata\local\temp\hash_code
> > to C:\users\uername\appdata\local\temp\hash_code, but under Windows file
> > system, these two path represents the same path,
> > so it's should not be FAILED.
> Then why not simply call os.path.normcase() on both paths before comparing
> them? That seems like a much cleaner solution to me.
>
That's why I ask you better solution, thanks.

>
> Cheers,
>
> Jelmer
> --
>
> https://code.launchpad.net/~luoyonggang/subvertpy/unittest-win32/+merge/96920
> You are the owner of lp:~luoyonggang/subvertpy/unittest-win32.
>

--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo