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

Revision history for this message
Yonggang Luo (luoyonggang) wrote :

2012/3/12 Yonggang Luo <email address hidden>

> 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.
>
It's not works, still return the path with incorrect case.

>
> >
> > 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
>
>
> https://code.launchpad.net/~luoyonggang/subvertpy/unittest-win32/+merge/96920
> You are the owner of lp:~luoyonggang/subvertpy/unittest-win32.
>

--
         此致

罗勇刚
Yours
    sincerely,
Yonggang Luo

« Back to merge proposal