Patchwork D6432: match: de-flake test-doctest.py by not depending on util.dirs() order

login
register
mail settings
Submitter phabricator
Date May 22, 2019, 9:26 p.m.
Message ID <differential-rev-PHID-DREV-xk6el3zrvd6e7givm2k5-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/40194/
State Superseded
Headers show

Comments

phabricator - May 22, 2019, 9:26 p.m.
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  util.dirs() yields directories in arbitrary order, which has made
  test-doctest.py flaky. I think they have been flaky since https://phab.mercurial-scm.org/rHGd8e55c0c642cf01292c335a6c3939cf024f1a73e
  (util: make util.dirs() and util.finddirs() include root directory
  (API), 2017-05-16). Before that commit, I think util.dirs() would
  return at most one entry, so there was only one iteration order. This
  patch fixes the problem by making _rootsdirsandparents() return a set
  (whose __str__() is defined to be in sorted order, I believe). The
  only caller wanted a set anyway.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6432

AFFECTED FILES
  mercurial/match.py

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel
Yuya Nishihara - May 22, 2019, 11:24 p.m.
> @@ -1384,26 +1384,26 @@
>      >>> _rootsdirsandparents(
>      ...     [(b'glob', b'g/h/*', b''), (b'glob', b'g/h', b''),
>      ...      (b'glob', b'g*', b'')])
> -    (['g/h', 'g/h', ''], [], ['', 'g'])
> +    (['g/h', 'g/h', ''], [], set(['', 'g']))

Perhaps, this would break py3 doctests. The repr syntax changed.
phabricator - May 22, 2019, 11:26 p.m.
yuja added a comment.


  > @@ -1384,26 +1384,26 @@
  > 
  >   >>> _rootsdirsandparents(
  >   ...     [(b'glob', b'g/h/*', b''), (b'glob', b'g/h', b''),
  >   ...      (b'glob', b'g*', b'')])
  > 
  > - (['g/h', 'g/h', ''], [], ['', 'g']) +    (['g/h', 'g/h', ''], [], set(['', 'g']))
  
  Perhaps, this would break py3 doctests. The repr syntax changed.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6432

To: martinvonz, #hg-reviewers
Cc: yuja, mercurial-devel
phabricator - May 23, 2019, 4:28 p.m.
martinvonz added a comment.


  In https://phab.mercurial-scm.org/D6432#93545, @yuja wrote:
  
  > > @@ -1384,26 +1384,26 @@
  > > 
  > >   >>> _rootsdirsandparents(
  > >   ...     [(b'glob', b'g/h/*', b''), (b'glob', b'g/h', b''),
  > >   ...      (b'glob', b'g*', b'')])
  > > 
  > > - (['g/h', 'g/h', ''], [], ['', 'g']) +    (['g/h', 'g/h', ''], [], set(['', 'g']))
  >
  > Perhaps, this would break py3 doctests. The repr syntax changed.
  
  
  Good point. However, it turns out they were already broken (b'' prefixes), so I'll leave it for the py3 folks to clean up.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6432

To: martinvonz, #hg-reviewers
Cc: yuja, mercurial-devel
Yuya Nishihara - May 24, 2019, 12:22 p.m.
>   > > @@ -1384,26 +1384,26 @@
>   > > 
>   > >   >>> _rootsdirsandparents(
>   > >   ...     [(b'glob', b'g/h/*', b''), (b'glob', b'g/h', b''),
>   > >   ...      (b'glob', b'g*', b'')])
>   > > 
>   > > - (['g/h', 'g/h', ''], [], ['', 'g']) +    (['g/h', 'g/h', ''], [], set(['', 'g']))
>   >
>   > Perhaps, this would break py3 doctests. The repr syntax changed.
>   
>   
>   Good point. However, it turns out they were already broken (b'' prefixes), so I'll leave it for the py3 folks to clean up.

FYI, there's a weird hack to normalize '', u'', and b'' because that's the
only way to make doctests portable.
phabricator - May 24, 2019, 12:28 p.m.
yuja added a comment.


  >   > > @@ -1384,26 +1384,26 @@
  >   > > 
  >   > >   >>> _rootsdirsandparents(
  >   > >   ...     [(b'glob', b'g/h/*', b''), (b'glob', b'g/h', b''),
  >   > >   ...      (b'glob', b'g*', b'')])
  >   > > 
  >   > > - (['g/h', 'g/h', ''], [], ['', 'g']) +    (['g/h', 'g/h', ''], [], set(['', 'g']))
  >   >
  >   > Perhaps, this would break py3 doctests. The repr syntax changed.
  >   
  >   
  >   Good point. However, it turns out they were already broken (b'' prefixes), so I'll leave it for the py3 folks to clean up.
  
  FYI, there's a weird hack to normalize '', u'', and b'' because that's the
  only way to make doctests portable.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6432

To: martinvonz, #hg-reviewers
Cc: yuja, mercurial-devel

Patch

diff --git a/mercurial/match.py b/mercurial/match.py
--- a/mercurial/match.py
+++ b/mercurial/match.py
@@ -614,7 +614,7 @@ 
         self._dirs = set(dirs)
         # parents are directories which are non-recursively included because
         # they are needed to get to items in _dirs or _roots.
-        self._parents = set(parents)
+        self._parents = parents
 
     def visitdir(self, dir):
         dir = normalizerootdir(dir, 'visitdir')
@@ -1384,26 +1384,26 @@ 
     >>> _rootsdirsandparents(
     ...     [(b'glob', b'g/h/*', b''), (b'glob', b'g/h', b''),
     ...      (b'glob', b'g*', b'')])
-    (['g/h', 'g/h', ''], [], ['', 'g'])
+    (['g/h', 'g/h', ''], [], set(['', 'g']))
     >>> _rootsdirsandparents(
     ...     [(b'rootfilesin', b'g/h', b''), (b'rootfilesin', b'', b'')])
-    ([], ['g/h', ''], ['', 'g'])
+    ([], ['g/h', ''], set(['', 'g']))
     >>> _rootsdirsandparents(
     ...     [(b'relpath', b'r', b''), (b'path', b'p/p', b''),
     ...      (b'path', b'', b'')])
-    (['r', 'p/p', ''], [], ['', 'p'])
+    (['r', 'p/p', ''], [], set(['', 'p']))
     >>> _rootsdirsandparents(
     ...     [(b'relglob', b'rg*', b''), (b're', b're/', b''),
     ...      (b'relre', b'rr', b'')])
-    (['', '', ''], [], [''])
+    (['', '', ''], [], set(['']))
     '''
     r, d = _patternrootsanddirs(kindpats)
 
-    p = []
-    # Append the parents as non-recursive/exact directories, since they must be
+    p = set()
+    # Add the parents as non-recursive/exact directories, since they must be
     # scanned to get to either the roots or the other exact directories.
-    p.extend(util.dirs(d))
-    p.extend(util.dirs(r))
+    p.update(util.dirs(d))
+    p.update(util.dirs(r))
 
     # FIXME: all uses of this function convert these to sets, do so before
     # returning.