Patchwork [4,of,6,V7] dirstate: add test for non-normal set consistency

login
register
mail settings
Submitter Laurent Charignon
Date Dec. 23, 2015, 9:20 p.m.
Message ID <968b926e9e546d7d09d7.1450905619@mbuchanan-mbp.DHCP.thefacebook.com>
Download mbox | patch
Permalink /patch/12327/
State Accepted
Headers show

Comments

Laurent Charignon - Dec. 23, 2015, 9:20 p.m.
# HG changeset patch
# User Laurent Charignon <lcharignon@fb.com>
# Date 1450744004 28800
#      Mon Dec 21 16:26:44 2015 -0800
# Node ID 968b926e9e546d7d09d7d68474b4c9235ce634a3
# Parent  dc11fff375efd02043a4f5e7ccce678a13c76b82
dirstate: add test for non-normal set consistency

This adds a test extension to check that the non-normal set contains the
expected entries. It wraps several methods of the dirstate to check that
the non-normal set has the correct values before and after the call. The
extension lives in contrib so that paranoid developers can easily
enable it to make sure that the non-normal set is consistent across more
complex operations than the included tests.
Augie Fackler - Dec. 31, 2015, 4:39 p.m.
On Wed, Dec 23, 2015 at 01:20:19PM -0800, Laurent Charignon wrote:
> # HG changeset patch
> # User Laurent Charignon <lcharignon@fb.com>
> # Date 1450744004 28800
> #      Mon Dec 21 16:26:44 2015 -0800
> # Node ID 968b926e9e546d7d09d7d68474b4c9235ce634a3
> # Parent  dc11fff375efd02043a4f5e7ccce678a13c76b82
> dirstate: add test for non-normal set consistency

Queued this whole series, thanks.

I do wonder if dirstatenonnormalcheck shouldn't just be something
hiding in devel-warnings or similar so that we can just turn it on
with a config knob, but if you agree with that idea we can do it as a
follow-up.

>
> This adds a test extension to check that the non-normal set contains the
> expected entries. It wraps several methods of the dirstate to check that
> the non-normal set has the correct values before and after the call. The
> extension lives in contrib so that paranoid developers can easily
> enable it to make sure that the non-normal set is consistent across more
> complex operations than the included tests.
>
> diff --git a/contrib/dirstatenonnormalcheck.py b/contrib/dirstatenonnormalcheck.py
> new file mode 100644
> --- /dev/null
> +++ b/contrib/dirstatenonnormalcheck.py
> @@ -0,0 +1,57 @@
> +# dirstatenonnormalcheck.py - extension to check the consistency of the
> +# dirstate's non-normal map
> +#
> +# For most operations on dirstate, this extensions checks that the nonnormalset
> +# contains the right entries.
> +# It compares the nonnormal file to a nonnormalset built from the map of all
> +# the files in the dirstate to check that they contain the same files.
> +
> +from __future__ import absolute_import
> +
> +from mercurial import (
> +    dirstate,
> +    extensions,
> +)
> +
> +def nonnormalentries(dmap):
> +    """Compute nonnormal entries from dirstate's dmap"""
> +    res = set()
> +    for f, e in dmap.iteritems():
> +        if e[0] != 'n' or e[3] == -1:
> +            res.add(f)
> +    return res
> +
> +def checkconsistency(ui, orig, dmap, _nonnormalset, label):
> +    """Compute nonnormalset from dmap, check that it matches _nonnormalset"""
> +    nonnormalcomputedmap = nonnormalentries(dmap)
> +    if _nonnormalset != nonnormalcomputedmap:
> +        ui.develwarn("%s call to %s\n" % (label, orig))
> +        ui.develwarn("inconsistency in nonnormalset\n")
> +        ui.develwarn("[nonnormalset] %s\n" % _nonnormalset)
> +        ui.develwarn("[map] %s\n" % nonnormalcomputedmap)
> +
> +def _checkdirstate(orig, self, arg):
> +    """Check nonnormal set consistency before and after the call to orig"""
> +    checkconsistency(self._ui, orig, self._map, self._nonnormalset, "before")
> +    r =  orig(self, arg)
> +    checkconsistency(self._ui, orig, self._map, self._nonnormalset, "after")
> +    return r
> +
> +def extsetup(ui):
> +    """Wrap functions modifying dirstate to check nonnormalset consistency"""
> +    dirstatecl = dirstate.dirstate
> +    devel = ui.configbool('devel', 'all-warnings')
> +    paranoid = ui.configbool('experimental', 'nonnormalparanoidcheck')
> +    if devel:
> +        extensions.wrapfunction(dirstatecl, '_writedirstate', _checkdirstate)
> +        if paranoid:
> +            # We don't do all these checks when paranoid is disable as it would
> +            # make the extension run very slowly on large repos
> +            extensions.wrapfunction(dirstatecl, 'normallookup', _checkdirstate)
> +            extensions.wrapfunction(dirstatecl, 'otherparent', _checkdirstate)
> +            extensions.wrapfunction(dirstatecl, 'normal', _checkdirstate)
> +            extensions.wrapfunction(dirstatecl, 'write', _checkdirstate)
> +            extensions.wrapfunction(dirstatecl, 'add', _checkdirstate)
> +            extensions.wrapfunction(dirstatecl, 'remove', _checkdirstate)
> +            extensions.wrapfunction(dirstatecl, 'merge', _checkdirstate)
> +            extensions.wrapfunction(dirstatecl, 'drop', _checkdirstate)
> diff --git a/tests/test-dirstate-nonnormalset.t b/tests/test-dirstate-nonnormalset.t
> new file mode 100644
> --- /dev/null
> +++ b/tests/test-dirstate-nonnormalset.t
> @@ -0,0 +1,22 @@
> +  $ cat >> $HGRCPATH << EOF
> +  > [ui]
> +  > logtemplate="{rev}:{node|short} ({phase}) [{tags} {bookmarks}] {desc|firstline}\n"
> +  > [extensions]
> +  > dirstateparanoidcheck = $TESTDIR/../contrib/dirstatenonnormalcheck.py
> +  > [experimental]
> +  > nonnormalparanoidcheck = True
> +  > [devel]
> +  > all-warnings=True
> +  > EOF
> +  $ mkcommit() {
> +  >    echo "$1" > "$1"
> +  >    hg add "$1"
> +  >    hg ci -m "add $1"
> +  > }
> +
> +  $ hg init testrepo
> +  $ cd testrepo
> +  $ mkcommit a
> +  $ mkcommit b
> +  $ mkcommit c
> +  $ hg status
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Matt Mackall - Jan. 2, 2016, 12:47 a.m.
On Thu, 2015-12-31 at 11:39 -0500, Augie Fackler wrote:
> On Wed, Dec 23, 2015 at 01:20:19PM -0800, Laurent Charignon wrote:
> > # HG changeset patch
> > # User Laurent Charignon <lcharignon@fb.com>
> > # Date 1450744004 28800
> > #      Mon Dec 21 16:26:44 2015 -0800
> > # Node ID 968b926e9e546d7d09d7d68474b4c9235ce634a3
> > # Parent  dc11fff375efd02043a4f5e7ccce678a13c76b82
> > dirstate: add test for non-normal set consistency
> 
> Queued this whole series, thanks.
> 
> I do wonder if dirstatenonnormalcheck shouldn't just be something
> hiding in devel-warnings or similar so that we can just turn it on
> with a config knob, but if you agree with that idea we can do it as a
> follow-up.

There were still a bunch of "nomal" symbol mis-namings in this series.

-- 
Mathematics is the supreme nostalgia of our time.

Patch

diff --git a/contrib/dirstatenonnormalcheck.py b/contrib/dirstatenonnormalcheck.py
new file mode 100644
--- /dev/null
+++ b/contrib/dirstatenonnormalcheck.py
@@ -0,0 +1,57 @@ 
+# dirstatenonnormalcheck.py - extension to check the consistency of the
+# dirstate's non-normal map
+#
+# For most operations on dirstate, this extensions checks that the nonnormalset
+# contains the right entries.
+# It compares the nonnormal file to a nonnormalset built from the map of all
+# the files in the dirstate to check that they contain the same files.
+
+from __future__ import absolute_import
+
+from mercurial import (
+    dirstate,
+    extensions,
+)
+
+def nonnormalentries(dmap):
+    """Compute nonnormal entries from dirstate's dmap"""
+    res = set()
+    for f, e in dmap.iteritems():
+        if e[0] != 'n' or e[3] == -1:
+            res.add(f)
+    return res
+
+def checkconsistency(ui, orig, dmap, _nonnormalset, label):
+    """Compute nonnormalset from dmap, check that it matches _nonnormalset"""
+    nonnormalcomputedmap = nonnormalentries(dmap)
+    if _nonnormalset != nonnormalcomputedmap:
+        ui.develwarn("%s call to %s\n" % (label, orig))
+        ui.develwarn("inconsistency in nonnormalset\n")
+        ui.develwarn("[nonnormalset] %s\n" % _nonnormalset)
+        ui.develwarn("[map] %s\n" % nonnormalcomputedmap)
+
+def _checkdirstate(orig, self, arg):
+    """Check nonnormal set consistency before and after the call to orig"""
+    checkconsistency(self._ui, orig, self._map, self._nonnormalset, "before")
+    r =  orig(self, arg)
+    checkconsistency(self._ui, orig, self._map, self._nonnormalset, "after")
+    return r
+
+def extsetup(ui):
+    """Wrap functions modifying dirstate to check nonnormalset consistency"""
+    dirstatecl = dirstate.dirstate
+    devel = ui.configbool('devel', 'all-warnings')
+    paranoid = ui.configbool('experimental', 'nonnormalparanoidcheck')
+    if devel:
+        extensions.wrapfunction(dirstatecl, '_writedirstate', _checkdirstate)
+        if paranoid:
+            # We don't do all these checks when paranoid is disable as it would
+            # make the extension run very slowly on large repos
+            extensions.wrapfunction(dirstatecl, 'normallookup', _checkdirstate)
+            extensions.wrapfunction(dirstatecl, 'otherparent', _checkdirstate)
+            extensions.wrapfunction(dirstatecl, 'normal', _checkdirstate)
+            extensions.wrapfunction(dirstatecl, 'write', _checkdirstate)
+            extensions.wrapfunction(dirstatecl, 'add', _checkdirstate)
+            extensions.wrapfunction(dirstatecl, 'remove', _checkdirstate)
+            extensions.wrapfunction(dirstatecl, 'merge', _checkdirstate)
+            extensions.wrapfunction(dirstatecl, 'drop', _checkdirstate)
diff --git a/tests/test-dirstate-nonnormalset.t b/tests/test-dirstate-nonnormalset.t
new file mode 100644
--- /dev/null
+++ b/tests/test-dirstate-nonnormalset.t
@@ -0,0 +1,22 @@ 
+  $ cat >> $HGRCPATH << EOF
+  > [ui]
+  > logtemplate="{rev}:{node|short} ({phase}) [{tags} {bookmarks}] {desc|firstline}\n"
+  > [extensions]
+  > dirstateparanoidcheck = $TESTDIR/../contrib/dirstatenonnormalcheck.py
+  > [experimental]
+  > nonnormalparanoidcheck = True
+  > [devel]
+  > all-warnings=True
+  > EOF
+  $ mkcommit() {
+  >    echo "$1" > "$1"
+  >    hg add "$1"
+  >    hg ci -m "add $1"
+  > }
+
+  $ hg init testrepo
+  $ cd testrepo
+  $ mkcommit a
+  $ mkcommit b
+  $ mkcommit c
+  $ hg status