[PATCH 2 of 2 STABLE resend] keyword: make status test after record and kwexpand/kwshrink reliable

[Christian Ebert]

* Matt Mackall on Thursday, September 08, 2011 at 16:21:43 -0500
> On Thu, 2011-09-08 at 21:59 +0100, Christian Ebert wrote:
>> * Matt Mackall on Thursday, September 08, 2011 at 14:16:55 -0500
>>> On Thu, 2011-09-08 at 19:33 +0100, Christian Ebert wrote:
>>>> # HG changeset patch
>>>> # User Christian Ebert <blacktrash at gmx.net>
>>>> # Date 1315506672 -3600
>>>> # Node ID 90dbf177b83584a5623fdc8d9dc297a5a869960a
>>>> # Parent  1e6056cf1f266f91c703c38dbb24a1b854752bbd
>>>> keyword: make status test after record and kwexpand/kwshrink reliable
>>>> 
>>>> This guarantees test failure when the dirstate code is omitted at
>>>> the end of the kwtemplater.overwrite method.
>>>> 
>>>> kwexpand/kwshrink:
>>>> Without a 1 second wait the test succeeds sometimes, even when
>>>> the dirstate of the overwritten file is not forced to normal.
>>>> 
>>>> record:
>>>> status after recording an added file allows to check whether
>>>> normallookup is needed after overwriting.
>>> 
>>> This looks highly suspect: we've gone to a great deal of effort to make
>>> these one second sleeps unnecessary. What's going on here?
>> 
>> Ignore my other confused answer. The problem is
>> repo.dirstate.normal() in connection with kwexpand/kwshrink.
>> 
>> The problem is that without sleep 1 the test succeeds
>> _sometimes_(!) when dirstate.normal() is omitted:
>> 
>> $ fgrep 'dirstate.normal(' ../hgext/keyword.py
>> $ ./run-tests.py test-keyword.t
>> 
>> --- /Users/chris/src/hg-stable/tests/test-keyword.t
>> +++ /Users/chris/src/hg-stable/tests/test-keyword.t.err
>> @@ -830,9 +830,12 @@
>>   $ hg --verbose kwshrink a
>>   overwriting a shrinking keywords
>>   $ hg status a
>> +  M a
>>   $ hg --verbose kwexpand a
>> -  overwriting a expanding keywords
>> +  abort: outstanding uncommitted changes
>> +  [255]
>>   $ hg status a
>> +  M a
>> 
>> kwexpand x/a should abort
>> 
>> @@ -841,8 +844,10 @@
>>   [255]
>>   $ cd x
>>   $ hg --debug commit -m xa -d '3 0' -u 'User Name <user at example.com>'
>> +  a
>>   x/a
>>    x/a: copy a:779c764182ce5d43e2b1eb66ce06d7b47bfe342e
>> +  overwriting a expanding keywords
>>   overwriting x/a expanding keywords
>>   committed changeset 3:b4560182a3f9a358179fd2d835c15e9da379c1e4
>>   $ cat a
>> 
>> ERROR: /Users/chris/src/hg-stable/tests/test-keyword.t output changed
>> !
>> Failed test-keyword.t: output changed
>> # Ran 1 tests, 0 skipped, 1 failed.
>> $ fgrep 'dirstate.normal(' ../hgext/keyword.py
>> $ ./run-tests.py test-keyword.t
>> .
>> # Ran 1 tests, 0 skipped, 0 failed.
>> 
>> 
>> The first failure is expected, but does not happen all the time.
>> The only way I found so far to make this fail reliable with dirstate.normal()
>> ommitted is sleep 1.
> 
> Can I get you to back up and explain how this bug manifests so those of
> us who understand dirstate but not keyword can grok it?

Sure. I'll try. First of all: it's not a bug, the extension does
the right thing. It is about making the test stricter in case
someone removes this or part of this code from the extension:

                if kwcmd:
                    self.repo.dirstate.normal(f)
                elif self.record:
                    self.repo.dirstate.normallookup(f)

In both cases (kwcmd means either kwexpand/kwshrink) we need to
force the dirstate to normal after a file containing keywords has
been overwritten. We have to cheat hg into believing the file in
question is not modified.

kwexpand/kwshrink:
1) check whether file is clean
2) overwrite file with keywords expanded/shrunk
3) hg now thinks (or should think) the file is modified
4) force dirstate of "modified" file to normal

record - wrt to files where all chunks have been recorded:
1) record changes
2) hg thinks file where all chunks have been recorded is clean
3) overwrite file with updated keywords
4) hg now thinks (or should think) that file is modified
5) force dirstate of overwritten "modified" file where all chunks
   have been recorded to normal

Whithout the changes to test-keyword.t the test would sometimes
succeed even when the dirstate fiddling was omitted. The change
to the test tries to prevent the introduction of a bug which is
currently not present.

The test enforces what hg "should think" and in the test
sometimes does not, i.e. in the test hg _sometimes_ thinks the
overwritten file is clean when it is actually not (yet).

Historically the extension did this dirstate tweaking also on
commit, but this is not needed anymore as expansion is now
wrapped into commitctx.

c
-- 
theatre - books - texts - movies
Black Trash Productions at home: http://www.blacktrash.org
Black Trash Productions on Facebook:
http://www.facebook.com/blacktrashproductions