This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6432

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

Authored by martinvonz on May 22 2019, 5:26 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHGc4b8f8637d7a: match: de-flake test-doctest.py by not depending on util.dirs() order
Summary

util.dirs() yields directories in arbitrary order, which has made
test-doctest.py flaky. I think they have been flaky since d8e55c0c642c
(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.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.May 22 2019, 5:26 PM
yuja added a subscriber: yuja.May 22 2019, 7:26 PM

@@ -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.

Closed by commit rHGc4b8f8637d7a: match: de-flake test-doctest.py by not depending on util.dirs() order (authored by martinvonz). · Explain WhyMay 22 2019, 7:41 PM
This revision was automatically updated to reflect the committed changes.
martinvonz added a comment.May 23 2019, 12:28 PM
In 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.

yuja added a comment.May 24 2019, 8:28 AM
> > @@ -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.

Revision Contents
Changeset List

PathPackages
Mmercurial/match.py (18 lines)

Diff 15241

mercurial/match.py

self._prefix = _prefix(kindpats) self._prefix = _prefix(kindpats)
roots, dirs, parents = _rootsdirsandparents(kindpats) roots, dirs, parents = _rootsdirsandparents(kindpats)
# roots are directories which are recursively included. # roots are directories which are recursively included.
self._roots = set(roots) self._roots = set(roots)
# dirs are directories which are non-recursively included. # dirs are directories which are non-recursively included.
self._dirs = set(dirs) self._dirs = set(dirs)
# parents are directories which are non-recursively included because # parents are directories which are non-recursively included because
# they are needed to get to items in _dirs or _roots. # they are needed to get to items in _dirs or _roots.
self._parents = set(parents) self._parents = parents
def visitdir(self, dir): def visitdir(self, dir):
dir = normalizerootdir(dir, 'visitdir') dir = normalizerootdir(dir, 'visitdir')
if self._prefix and dir in self._roots: if self._prefix and dir in self._roots:
return 'all' return 'all'
return (dir in self._roots or return (dir in self._roots or
dir in self._dirs or dir in self._dirs or
dir in self._parents or dir in self._parents or
be matched non-recursively, and `parents` are the implicitly required be matched non-recursively, and `parents` are the implicitly required
directories to walk to items in either roots or dirs. directories to walk to items in either roots or dirs.
Returns a tuple of (roots, dirs, parents). Returns a tuple of (roots, dirs, parents).
>>> _rootsdirsandparents( >>> _rootsdirsandparents(
... [(b'glob', b'g/h/*', b''), (b'glob', b'g/h', b''), ... [(b'glob', b'g/h/*', b''), (b'glob', b'g/h', b''),
... (b'glob', b'g*', b'')]) ... (b'glob', b'g*', b'')])
(['g/h', 'g/h', ''], [], ['', 'g']) (['g/h', 'g/h', ''], [], set(['', 'g']))
>>> _rootsdirsandparents( >>> _rootsdirsandparents(
... [(b'rootfilesin', b'g/h', b''), (b'rootfilesin', b'', b'')]) ... [(b'rootfilesin', b'g/h', b''), (b'rootfilesin', b'', b'')])
([], ['g/h', ''], ['', 'g']) ([], ['g/h', ''], set(['', 'g']))
>>> _rootsdirsandparents( >>> _rootsdirsandparents(
... [(b'relpath', b'r', b''), (b'path', b'p/p', b''), ... [(b'relpath', b'r', b''), (b'path', b'p/p', b''),
... (b'path', b'', b'')]) ... (b'path', b'', b'')])
(['r', 'p/p', ''], [], ['', 'p']) (['r', 'p/p', ''], [], set(['', 'p']))
>>> _rootsdirsandparents( >>> _rootsdirsandparents(
... [(b'relglob', b'rg*', b''), (b're', b're/', b''), ... [(b'relglob', b'rg*', b''), (b're', b're/', b''),
... (b'relre', b'rr', b'')]) ... (b'relre', b'rr', b'')])
(['', '', ''], [], ['']) (['', '', ''], [], set(['']))
''' '''
r, d = _patternrootsanddirs(kindpats) r, d = _patternrootsanddirs(kindpats)
p = [] p = set()
# Append the parents as non-recursive/exact directories, since they must be # Add the parents as non-recursive/exact directories, since they must be
# scanned to get to either the roots or the other exact directories. # scanned to get to either the roots or the other exact directories.
p.extend(util.dirs(d)) p.update(util.dirs(d))
p.extend(util.dirs(r)) p.update(util.dirs(r))
# FIXME: all uses of this function convert these to sets, do so before # FIXME: all uses of this function convert these to sets, do so before
# returning. # returning.
# FIXME: all uses of this function do not need anything in 'roots' and # FIXME: all uses of this function do not need anything in 'roots' and
# 'dirs' to also be in 'parents', consider removing them before returning. # 'dirs' to also be in 'parents', consider removing them before returning.
return r, d, p return r, d, p
def _explicitfiles(kindpats): def _explicitfiles(kindpats):