[rfc] known failure for all tests that fails because of OS locks @ win32

Martin Pool mbp at sourcefrog.net
Thu Apr 19 15:30:09 BST 2007


If we don't fix them properly then marking them as known failures
before the release would be good.

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

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.

So I suppose that's why the except block tries to release only in case
of an error, but that's not quite correct.

The most correct thing is probably to replace the test's except block with

  finally:
    if state._lock_token:
      state.unlock()

But aside from that I don't see why this test would be a problem - I
can't see where it would be trying to lock the dirstate while it's
read locked.  I've stepped through it with pdb on Linux and can't see
it.  If you could tell me which line the error comes from that would
help.

In check_state_with_reopen:

        del state # Callers should unlock

not sure what that does, probably nothing...

-- 
Martin



More information about the bazaar mailing list