This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9024

run-test: allow relative path in `--blacklist` and `--whitelist` (issue6351)
ClosedPublic

Authored by acezar on Sep 17 2020, 4:57 AM.

Details

Reviewers
Alphare
pulkit
Group Reviewers
hg-reviewers
Commits
rHG22140fd783d2: run-test: allow relative path in `--blacklist` and `--whitelist` (issue6351)
rHG2951c0fe9d16: run-test: allow relative path in `--blacklist` and `--whitelist` (issue6351)
Summary

When specifying a test with --blacklist or --whitelist with path relatives
to the repository root (eg: tests/test-check-commit.t) the file is not taken
into account. It only works when the name of the test is given.

It would be better if --blacklist and --whitelist behaviors where compatible
with --test-list.

This patch allows to use relative path with --blacklist and --whitelist
while staying compatible with the old behavior by checking the test relative
path in addition to its name.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

acezar created this revision.Sep 17 2020, 4:57 AM
Alphare accepted this revision.Sep 17 2020, 8:02 AM
pulkit added a subscriber: pulkit.Sep 17 2020, 8:43 AM
pulkit added inline comments.
tests/run-tests.py
1002

We can just have this as another class variable.

pulkit added a comment.Sep 17 2020, 8:45 AM

Missed adding, for bug fixes, we specify bug id at end of commit message. https://www.mercurial-scm.org/wiki/ContributingChanges#Patch_descriptions

acezar retitled this revision from run-test: make `--blacklist` and `--whitelist` behave like `--test-list` to run-test: allow relative path in `--blacklist` and `--whitelist` (issue6351).Sep 17 2020, 11:49 AM
acezar edited the summary of this revision. (Show Details)
acezar updated this revision to Diff 22677.
acezar marked an inline comment as done.Sep 18 2020, 4:32 AM
acezar edited the summary of this revision. (Show Details)
acezar updated this revision to Diff 22716.
acezar added a comment.Sep 18 2020, 4:34 AM
In D9024#135365, @pulkit wrote:

Missed adding, for bug fixes, we specify bug id at end of commit message. https://www.mercurial-scm.org/wiki/ContributingChanges#Patch_descriptions

Improved the commit message.

pulkit accepted this revision.Sep 18 2020, 10:43 AM
This revision is now accepted and ready to land.Sep 18 2020, 10:43 AM
martinvonz added a subscriber: martinvonz.Sep 21 2020, 1:15 PM
martinvonz added inline comments.
tests/run-tests.py
2410–2411

Black wanted these two lines to be joined. I'll fix it in flight.

Revision Contents
Changeset List

PathPackages
Mtests/run-tests.py (17 lines)

Diff 22659

tests/run-tests.py

self._ret = None self._ret = None
self._out = None self._out = None
self._skipped = None self._skipped = None
self._testtmp = None self._testtmp = None
self._chgsockdir = None self._chgsockdir = None
self._refout = self.readrefout() self._refout = self.readrefout()
@property
pulkitUnsubmitted
Done

We can just have this as another class variable.

pulkit: We can just have this as another class variable.
def relpath(self):
return os.path.relpath(self.path)
def readrefout(self): def readrefout(self):
"""read reference output""" """read reference output"""
# If we're not in --debug mode and reference output file exists, # If we're not in --debug mode and reference output file exists,
# check test output against it. # check test output against it.
if self._debug: if self._debug:
return None # to match "out is None" return None # to match "out is None"
elif os.path.exists(self.refpath): elif os.path.exists(self.refpath):
with open(self.refpath, 'rb') as f: with open(self.refpath, 'rb') as f:
if getattr(test, 'should_reload', False): if getattr(test, 'should_reload', False):
return self._loadtest(test, num_tests[0]) return self._loadtest(test, num_tests[0])
return test return test
if not os.path.exists(test.path): if not os.path.exists(test.path):
result.addSkip(test, "Doesn't exist") result.addSkip(test, "Doesn't exist")
continue continue
if not (self._whitelist and test.bname in self._whitelist): is_whitelisted = self._whitelist and (
if self._blacklist and test.bname in self._blacklist: test.relpath in self._whitelist
or test.bname in self._whitelist
)
if not is_whitelisted:
is_blacklisted = self._blacklist and (
test.relpath in self._blacklist
or test.bname in self._blacklist
martinvonzUnsubmitted
Not Done

Black wanted these two lines to be joined. I'll fix it in flight.

martinvonz: Black wanted these two lines to be joined. I'll fix it in flight.
)
if is_blacklisted:
result.addSkip(test, 'blacklisted') result.addSkip(test, 'blacklisted')
continue continue
if self._keywords: if self._keywords:
with open(test.path, 'rb') as f: with open(test.path, 'rb') as f:
t = f.read().lower() + test.bname.lower() t = f.read().lower() + test.bname.lower()
ignored = False ignored = False
for k in self._keywords.lower().split(): for k in self._keywords.lower().split():
if k not in t: if k not in t:
result.addIgnore(test, "doesn't match keyword") result.addIgnore(test, "doesn't match keyword")
ignored = True ignored = True