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.

Download Raw Diff

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

Herald added a reviewer: hg-reviewers. · View Herald TranscriptMay 22 2019, 5:26 PM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

martinvonz added a child revision: D6433: match: remove an obsolete comment about util.finddirs().May 22 2019, 5: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.

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.

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

			Path	Packages
M			mercurial/match.py (18 lines)

	Status	Author	Revision
	Closed	martinvonz	D6433 match: remove an obsolete comment about util.finddirs()
	Closed	martinvonz	D6432 match: de-flake test-doctest.py by not depending on util.dirs() order

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):

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	15234		May 22 2019, 5:26 PM	★	★
Diff 2	15241	rHGc4b8f8637d7a2dde1575d47b7ca9e26eb4a49eff	May 22 2019, 4:58 PM	★	★