Skip to content

Commit

Permalink
Fix lint of redundant virtual expectations for glob expectations
Browse files Browse the repository at this point in the history
For the following case:
  bar/test.html [ Failure ]
  virtual/foo/bar/* [ Pass ]
  virtual/foo/bar/test.html [ Failure ]
previously the last virtual expectation was mistakenly reported as
redundant.

Now ignore such entries if there is a virtual glob expectation
overriding the base expectation.

Bug: 1124787
Change-Id: I9ab0c751343cf066e5c8f2db803a610e0cbb27e1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2393945
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Auto-Submit: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Robert Ma <robertma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805052}
GitOrigin-RevId: d8026cdeb126c20e4f9961409c080a66c422c0a6
  • Loading branch information
wangxianzhu authored and Copybara-Service committed Sep 8, 2020
1 parent c4bc246 commit 92d9e98
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 14 deletions.
36 changes: 23 additions & 13 deletions blink/tools/blinkpy/web_tests/lint_test_expectations.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,24 +217,34 @@ def _check_redundant_virtual_expectations(host, port, path, expectations):
return []

failures = []
expectations_by_test = {}
for exp in expectations:
if exp.test:
expectations_by_test.setdefault(exp.test, []).append(exp)

base_expectations_by_test = {}
virtual_expectations = []
virtual_globs = []
for exp in expectations:
if not exp.test:
continue

base_test = port.lookup_virtual_test_base(exp.test)
if not base_test:
continue
if base_test:
virtual_expectations.append((exp, base_test))
if exp.is_glob:
virtual_globs.append(exp.test[:-1])
else:
base_expectations_by_test.setdefault(exp.test, []).append(exp)

for base_exp in expectations_by_test.get(base_test, []):
for (exp, base_test) in virtual_expectations:
for base_exp in base_expectations_by_test.get(base_test, []):
if (base_exp.results == exp.results
and base_exp.is_slow_test == exp.is_slow_test
and base_exp.tags.issubset(exp.tags)
and base_exp.reason == exp.reason):
and base_exp.reason == exp.reason
# Don't report redundant expectation in the following case
# bar/test.html [ Failure ]
# virtual/foo/bar/* [ Pass ]
# virtual/foo/bar/test.html [ Failure ]
# For simplicity, tags of the glob expectations are ignored.
and not any(exp.test != glob and exp.test.startswith(glob)
for glob in virtual_globs)):
error = "{}:{} Expectation '{}' is redundant with '{}' in line {}".format(
host.filesystem.basename(path), exp.lineno, exp.test,
base_test, base_exp.lineno)
Expand Down Expand Up @@ -314,10 +324,10 @@ def _check_non_wpt_in_android_override(host, port, path, expectations):
failures = []
for exp in expectations:
if exp.test and not port.is_wpt_test(exp.test):
error = "{}:{} Expectation '{}' is for a non WPT test".format(
host.filesystem.basename(path), exp.lineno, exp.to_string())
failures.append(error)
_log.error(error)
error = "{}:{} Expectation '{}' is for a non WPT test".format(
host.filesystem.basename(path), exp.lineno, exp.to_string())
failures.append(error)
_log.error(error)
return failures


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,10 @@ def test_virtual_test_redundant_expectation(self):
'[ win ] virtual/foo/test/test1.html [ Failure ]\n'
'[ mac release ] virtual/foo/test/test1.html [ Pass ]\n'
'test/test2.html [ Failure ]\n'
'crbug.com/1234 virtual/foo/test/test2.html [ Failure ]')
'crbug.com/1234 virtual/foo/test/test2.html [ Failure ]\n'
'test/subtest/test2.html [ Failure ]\n'
'virtual/foo/test/subtest/* [ Pass ]\n'
'virtual/foo/test/subtest/test2.html [ Failure ]')
port.expectations_dict = lambda: {
'testexpectations': test_expectations
}
Expand Down

0 comments on commit 92d9e98

Please sign in to comment.