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

> 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.

Cheers,

Jelmer