Skip to content

Commit

Permalink
Fix presubmit error for expired virtual test suites
Browse files Browse the repository at this point in the history
There are presubmit checks to ensure virtual baselines/expectations are
deleted once a VTS is deleted. With the introduction of expiry dates for
virtual suites, this can be triggered once a VTS expired.

Do not trigger such presubmit check error on an expired VTS. Delay
that to when the VTS is actually deleted from the json file.

For expired VTSs, plan to have a service to automatically delete
them after some amount of time.

Bug: 1450585
Change-Id: Ibf8179ad0f372860e0518607b9955c12a6f01cc7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4595556
Reviewed-by: David Baron <dbaron@chromium.org>
Commit-Queue: Weizhong Xia <weizhong@google.com>
Reviewed-by: Jonathan Lee <jonathanjlee@google.com>
Cr-Commit-Position: refs/branch-heads/5790@{#420}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
WeizhongX authored and Chromium LUCI CQ committed Jun 6, 2023
1 parent 0071aa6 commit 351e16a
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 34 deletions.
Expand Up @@ -66,6 +66,8 @@ def lint(host, options):
# Force a manifest update to ensure it's always up-to-date.
# TODO(crbug.com/1411505): See if the manifest refresh can be made faster.
options.manifest_update = True
# Not a presubmit error for stale test expectations due to expired VTS
options.include_expired = True
port = host.port_factory.get(options=options)

# Add all extra expectation files to be linted.
Expand Down
65 changes: 33 additions & 32 deletions third_party/blink/tools/blinkpy/web_tests/port/base.py
Expand Up @@ -285,7 +285,6 @@ def __init__(self, host, port_name, options=None, **kwargs):
self.set_option_default('wpt_only', False)
self._test_configuration = None
self._results_directory = None
self._virtual_test_suites = None
self._used_expectation_files = None

def __str__(self):
Expand Down Expand Up @@ -2308,38 +2307,40 @@ def look_for_new_samples(self, unresponsive_processes, start_time):
def sample_process(self, name, pid):
pass

@memoized
def virtual_test_suites(self):
if self._virtual_test_suites is None:
path_to_virtual_test_suites = self._filesystem.join(
self.web_tests_dir(), 'VirtualTestSuites')
assert self._filesystem.exists(path_to_virtual_test_suites), \
path_to_virtual_test_suites + ' not found'
try:
test_suite_json = json.loads(
self._filesystem.read_text_file(
path_to_virtual_test_suites))
self._virtual_test_suites = []
current_time = datetime.now()
for json_config in test_suite_json:
# Strings are treated as comments.
if isinstance(json_config, str):
continue
expires = json_config.get('expires', 'never')
if (expires.lower() != 'never' and datetime.strptime(
expires, '%b %d, %Y') <= current_time):
# do not load expired virtual suites
continue
vts = VirtualTestSuite(**json_config)
if any(vts.full_prefix == s.full_prefix
for s in self._virtual_test_suites):
raise ValueError(
'{} contains entries with the same prefix: {!r}. Please combine them'
.format(path_to_virtual_test_suites, json_config))
self._virtual_test_suites.append(vts)
except ValueError as error:
raise ValueError('{} is not a valid JSON file: {}'.format(
path_to_virtual_test_suites, error))
return self._virtual_test_suites
include_expired = self.get_option('include_expired', False)
path_to_virtual_test_suites = self._filesystem.join(
self.web_tests_dir(), 'VirtualTestSuites')
assert self._filesystem.exists(path_to_virtual_test_suites), \
path_to_virtual_test_suites + ' not found'
virtual_test_suites = []
try:
test_suite_json = json.loads(
self._filesystem.read_text_file(path_to_virtual_test_suites))
current_time = datetime.now()
for json_config in test_suite_json:
# Strings are treated as comments.
if isinstance(json_config, str):
continue
expires = json_config.get('expires', 'never')
expired = (expires.lower() != 'never' and datetime.strptime(
expires, '%b %d, %Y') <= current_time)
if expired and not include_expired:
# Do not include expired virtual suites, except when requested (such as
# for presubmit checks).
continue
vts = VirtualTestSuite(**json_config)
if any(vts.full_prefix == s.full_prefix
for s in virtual_test_suites):
raise ValueError(
'{} contains entries with the same prefix: {!r}. Please combine them'
.format(path_to_virtual_test_suites, json_config))
virtual_test_suites.append(vts)
except ValueError as error:
raise ValueError('{} is not a valid JSON file: {}'.format(
path_to_virtual_test_suites, error))
return virtual_test_suites

def _all_virtual_tests(self, tests_by_dir):
tests = []
Expand Down
5 changes: 3 additions & 2 deletions third_party/blink/web_tests/PRESUBMIT.py
Expand Up @@ -255,9 +255,10 @@ def _CheckForExtraVirtualBaselines(input_api, output_api):

from blinkpy.common.host import Host
port_factory = Host().port_factory
port = port_factory.get(port_factory.all_port_names()[0])
port.set_option_default('include_expired', True)
known_virtual_suites = [
suite.full_prefix[8:-1] for suite in port_factory.get(
port_factory.all_port_names()[0]).virtual_test_suites()
suite.full_prefix[8:-1] for suite in port.virtual_test_suites()
]

results = []
Expand Down

0 comments on commit 351e16a

Please sign in to comment.