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

On 03/12/2012 02:46 AM, 罗勇刚(Yonggang Luo) wrote:
>
>
> 2012/3/12 Jelmer Vernooij <<email address hidden> <mailto:<email address hidden>>>
>
> Am 12/03/12 02:10, schrieb Yonggang Luo:
>
> 2012/3/12 Yonggang Luo<<email address hidden>
> <mailto:<email address hidden>>>
>
> 2012/3/12 Jelmer Vernooij<<email address hidden>
> <mailto:<email address hidden>>>
>
> 2012/3/11 Jelmer Vernooij<<email address hidden>
> <mailto:<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.
>
> Can you please elaborate? What doesn't work ? Does
> os.path.normcase not return a normalized case?
>
> It's didn't change the result, just act with os.path.normcase 's
> documented case.
> The path is return without change except \ are replaced with /.
>
It sounds like you're using os.path.normpath, not os.path.normcase.

Jelmer