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

Matt Mackall mpm at selenic.com
Fri Sep 9 15:02:33 CDT 2011


On Fri, 2011-09-09 at 10:40 +0100, Christian Ebert wrote:
> * 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.

I see. Ok, please resend your patch.

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list