[MERGE] update for dirstate.initialize test

[John Arbash Meinel]

Alexander Belchenko wrote:
> Martin Pool ?8H5B:
>> 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
> 
> This test fails during execution self.assertFileEqual()
> 
> ERROR: #353 test_initialize (bzrlib.tests.test_dirstate.TestDirStateInitialize)
> 
> vvvv[log from bzrlib.tests.test_dirstate.TestDirStateInitialize.test_initialize]
> trying to create missing lock 'C:/Temp/selftest.win32/test0000.tmp/0K/00353/work/dirstate'
> 
> ^^^^[log from bzrlib.tests.test_dirstate.TestDirStateInitialize.test_initialize]
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "C:\Temp\selftest.win32\bzrlib\tests\test_dirstate.py", line 652, in test_initialize
>     'dirstate')
>   File "C:\Temp\selftest.win32\bzrlib\tests\__init__.py", line 1925, in assertFileEqual
>     s = f.read()
> IOError: [Errno 13] Permission denied
> 
> 
> The dirstate file is write-locked in this moment
> and read operation on file raise exception.
> 
>> 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...

Actually, this is a bad comment. It *used* to be that callers should
unlock. But because the 'check_state_with_reopen' is reading state, it
needs to be passed in locked, and then we want to open it again, so it
needs to unlock it.

I changed the lines to:
        try:
            self.assertIsInstance(state, dirstate.DirState)
            lines = state.get_lines()
            self.assertFileEqual(''.join(state.get_lines()),
                'dirstate')
        except:
            state.unlock()
            raise
        self.check_state_with_reopen(expected_result, state)


Also, my understanding of the failure was incorrect. It seems that on
windows, if you open the same file 2 times, and lock one of the handles,
you cannot read from the other handle. So my final rewrite is:

    def test_initialize(self):
        expected_result = ([], [
            (('', '', 'TREE_ROOT'), # common details
             [('d', '', 0, False, dirstate.DirState.NULLSTAT), # current
tree
             ])
            ])
        state = dirstate.DirState.initialize('dirstate')
        try:
            self.assertIsInstance(state, dirstate.DirState)
            lines = state.get_lines()
        finally:
            state.unlock()
        self.assertFileEqual(''.join(lines), 'dirstate')
        state.lock_read() # check_state_with_reopen will unlock
        self.check_state_with_reopen(expected_result, state)

And this is fixed in the attached bundle.

I checked on my win32 machine and it passes. Do you want to confirm?

John
=:->
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: dirstate_initialize.patch
Url: https://lists.ubuntu.com/archives/bazaar/attachments/20070419/56150eb9/attachment-0001.diff