[merge] win32- more test suite fixes

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Aaron Bentley wrote:
> John Arbash Meinel wrote:
>>> Aaron Bentley wrote:
>>> I suppose we could wrap the 'fancy_rename()' in a try/catch and do:
>>>
>>> try:
>>>   fancy_rename(...)
>>> except OSError, e:
>>>   if e.errno == errno.EPERM:
>>>     os.lstat(old)
>>>   raise
>>>
>>> That should translate an EPERM into a ENOENT when appropriate, without
>>> the extra overhead.
> 
> That's what I had in mind, but if you'd rather do it the simpler way, okay.
> 

I went ahead and did this. I was probably pessimizing the common case
more than I needed to. It seems we also EACCES instead of ENOENT for
os.rename, so I check for both EPERM to handle renaming to the current
working dir, and EACCES just to translate it into ENOENT.


>>>>> What bugs me is that if we enter the finally block due to an exception,
>>>>> and then self.tt.finalize throws, we'll never see the original
>>>>> exception.  I'm fine with doing it in this case, but we should find a
>>>>> more general solution.
> 
>>> Well, I can restore what you originally had.
> 
> No, your change is definitely an improvement.
> 
>>> Or I could just restore it to the old code. I removed it to track down a
>>> different bug, which is now fixed, so I don't *need* to see the tt
>>> exceptions.
> 
> No, as I said, I'm fine with doing it in this case.  Generally, though,
> I think we should look at having an Exception class that bundles
> multiple exceptions together.

Something to think about. I think even more helpful when python adds
tracebacks *to* the exception objects. I believe I read a PEP on it, and
it might be in 2.5.

...

> My feeling is that a test shouldn't pass unless it was run to completion
> successfully.  E.g. a test that cannot be performed on Windows may still
> be a failing test, and should be run on another platform to ensure that
> it passes.
> 
> I can think of three alternatives to pass/fail.
> 1. expected failure
> 2. cannot be performed on this platform
> 3. requirements not satisfied
> 
> 1 can be considered a 'pass', in a sense.  It doesn't do what we want,
> but it does what we're expecting.  At least it didn't fail for a
> different reason!

Right. I'm doing quite a bit of this for old formats and win32. But in
general I do a lot of checking to make sure the conditions are correct
for the given error.
Like we are getting LockErrors when creating 2 repository objects in the
same place on win32 with pre-split-out branches. This is a known error
that we aren't going to fix (meta-branches somehow do better about this).


> 
> 2 means 'please rerun me on another platform'
> 
> 3 means 'please install (paramiko|pycurl|...)'
> 
>>> If self.branch is None, that means we couldn't setup the test. Primarily
>>> because we are on Win32, and one of the files is using <> in the
>>> file_id, which we didn't escape before knits.
> 
> Oh, right.
> 

There are some comments as to this effect. It is much clearer in source
form than in the diff.


...

> 
>>>  Also, with your proposal, we will
>>> double-cleanup in the case that segment in contents raises.
> 
> Drat.  Yeah, you got me.
> 

...

> 
> TreeTransform tends that way anyhow.  Ugly, but right.
> 
> Aaron

Sure, I'll go ahead and fix it the right way.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD4DBQFEpWM8JdeBCYSNAAMRAqt5AKCBUzOxlsyl92/yNTh9RHeOIbx5pgCXZ9dg
3Dz7dHKXOR6Zbmr/A+W5Bg==
=yg6x
-----END PGP SIGNATURE-----