[rfc] known failure for all tests that fails because of OS locks @ win32
John Arbash Meinel
john at arbash-meinel.com
Thu Apr 19 18:19:35 BST 2007
Martin Pool wrote:
> If we don't fix them properly then marking them as known failures
> before the release would be good.
Ultimately I wanted to make OS Locks exclusive on Linux, so I was hoping
to have tests updated to not fail, since they should fail on other
platforms as well.
Obviously I didn't have enough time to fix all of them, though. So
short-term knownFailure is ok.
>
>> Most of them used pattern try to create reference WT with read lock
>> after they created testing WT with write lock. Because on win32
>> such locks is mutually exclusive I'm give up to fix them.
>>
>> May I mark all this tests as known failure @ win32?
>
> I had a look at TestDirStateInitialize, which seemed like the simplest one:
>
> 639 class TestDirStateInitialize(TestCaseWithDirState):
> 640
> 641 def test_initialize(self):
> 642 expected_result = ([], [
> 643 (('', '', 'TREE_ROOT'), # common details
> 644 [('d', '', 0, False, dirstate.DirState.NULLSTAT), #
> current tree
> 645 ])
> 646 ])
> 647 state = dirstate.DirState.initialize('dirstate')
> 648 try:
> 649 self.assertIsInstance(state, dirstate.DirState)
> 650 lines = state.get_lines()
> 651 self.assertFileEqual(''.join(state.get_lines()),
> 652 'dirstate')
> 653 self.check_state_with_reopen(expected_result, state)
> 654 except:
> 655 state.unlock()
> 656 raise
>
> First, I don't see why we're unlocking from an except block not a
> finally block. The test always ought to do that to clean up.
I think it is because we want to return a Write-locked state if
everything succeeds.
IIRC it is because the tests are cleaner/easier to write if the locking
is taken care of with "state.lock()" and "self.addCleanup(state.unlock)"
The reason for 'initialize' to leave it locked is because you almost
always need it that way, and it needs it locked to initialize. So adding
an extra unlock just to have callers always lock seemed foolish.
>
> It seems a bit strange that the check_state_with_reopen is inside this
> block that holds the lock. check_state_with_unlock releases the lock
> as a side effect, which makes it hard to get the calling code's
> cleanup right. I suppose that wouldn't normally matter in tests where
> we don't care about handling unexpected errors but it might be
> complicating things here.
The
try/except/raise
Is because we know the 'state' is locked, and we don't want to
complicate the test failure by having the "Lock not released"
warning/error cluttering things up.
It would be possible to always unlock, and then lock before we return.
And certainly if we are doing a read lock during a write lock, we should
do that anyway.
John
=:->
More information about the bazaar
mailing list