Skip to content

Commit

Permalink
Revert "[src] Add inverse filter file generation for RTS/Quick Run"
Browse files Browse the repository at this point in the history
This reverts commit 43071db.

Reason for revert: Compile has started failing for Quick Runs

Original change's description:
> [src] Add inverse filter file generation for RTS/Quick Run
>
> When RTS is enabled this will additionally generate the inverted
> filter file (a file that will only run the skipped tests), add it to
> the isolate and generates additional commands to run with these
> inverted filter files.
>
> See the design doc for more detailed information:
> http://docs/document/d/14XNntG5FNhf_lyTLtc1XlU2ZfiKmv8NzJK87D1m6zBE?resourcekey=0-GrHDLuC_iRkseeaEfGI-cQ
>
> led build using this CL:
> https://ci.chromium.org/raw/build/logs.chromium.org/chromium/led/sshrimp_google.com/15464dab518d5d7b92932010409bb216b5d47e7a695691fe016448e8247be9c6/+/build.proto?server=chromium-swarm.appspot.com
>
> Bug: 1344885
> Change-Id: Ic438bbb44495d2eb6d80e653f68a747d8f70e170
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3765373
> Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
> Reviewed-by: Ben Pastene <bpastene@chromium.org>
> Commit-Queue: Struan Shrimpton <sshrimp@google.com>
> Cr-Commit-Position: refs/heads/main@{#1034546}

Bug: 1344885
Change-Id: I5695f77d92d83a1acd29c6a86d204ad7c670488c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3828900
Auto-Submit: Struan Shrimpton <sshrimp@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Ben Pastene <bpastene@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1035226}
  • Loading branch information
Struan Shrimpton authored and Chromium LUCI CQ committed Aug 15, 2022
1 parent 6496a4d commit aa30199
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 84 deletions.
21 changes: 6 additions & 15 deletions build/add_rts_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,10 @@
# Copyright (c) 2021 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
"""Creates a dummy RTS filter file and a dummy inverse filter file if a
real ones do not exist yet. Real filter files (and their inverse) are
generated by the RTS binary for suites with any skippable tests. The
rest of the suites need to have dummy files because gn will expect the
file to be present.
"""Creates a dummy RTS filter file if a real one doesn't exist yes.
Real filter files are generated by the RTS binary for suites with any
skippable tests. The rest of the suites need to have dummy files because gn
will expect the file to be present.
Implementation uses try / except because the filter files are written
relatively close to when this code creates the dummy files.
Expand All @@ -23,15 +22,6 @@

def main():
filter_file = sys.argv[1]
# '*' is a dummy that means run everything
write_filter_file(filter_file, '*')

inverted_filter_file = sys.argv[1]
# '-*' is a dummy that means run nothing
write_filter_file(inverted_filter_file, '-*')


def write_filter_file(filter_file, filter_string):
directory = os.path.dirname(filter_file)
try:
os.makedirs(directory)
Expand All @@ -40,6 +30,7 @@ def write_filter_file(filter_file, filter_string):
pass
else:
raise

try:
fp = os.open(filter_file, os.O_CREAT | os.O_EXCL | os.O_WRONLY)
except OSError as err:
Expand All @@ -49,7 +40,7 @@ def write_filter_file(filter_file, filter_string):
raise
else:
with os.fdopen(fp, 'w') as file_obj:
file_obj.write(filter_string)
file_obj.write('*') # '*' is a dummy that means run everything


if __name__ == '__main__':
Expand Down
12 changes: 2 additions & 10 deletions build/config/android/rules.gni
Original file line number Diff line number Diff line change
Expand Up @@ -3885,16 +3885,8 @@ if (enable_java_templates) {
action("${invoker.target_name}__rts_filters") {
script = "//build/add_rts_filters.py"
rts_file = "${root_build_dir}/gen/rts/${invoker.target_name}.filter"
inverted_rts_file =
"${root_build_dir}/gen/rts/${invoker.target_name}_inverted.filter"
args = [
rebase_path(rts_file, root_build_dir),
rebase_path(inverted_rts_file, root_build_dir),
]
outputs = [
rts_file,
inverted_rts_file,
]
args = [ rebase_path(rts_file, root_build_dir) ]
outputs = [ rts_file ]
}
}
_incremental_apk = !(defined(invoker.never_incremental) &&
Expand Down
24 changes: 4 additions & 20 deletions testing/test.gni
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,8 @@ template("test") {
action("${target_name}__rts_filters") {
script = "//build/add_rts_filters.py"
rts_file = "${root_build_dir}/gen/rts/${invoker.target_name}.filter"
inverted_rts_file =
"${root_build_dir}/gen/rts/${invoker.target_name}_inverted.filter"
args = [
rebase_path(rts_file, root_build_dir),
rebase_path(inverted_rts_file, root_build_dir),
]
outputs = [
rts_file,
inverted_rts_file,
]
args = [ rebase_path(rts_file, root_build_dir) ]
outputs = [ rts_file ]
}
}

Expand Down Expand Up @@ -1042,16 +1034,8 @@ template("script_test") {
action("${target_name}__rts_filters") {
script = "//build/add_rts_filters.py"
rts_file = "${root_build_dir}/gen/rts/${invoker.target_name}.filter"
inverted_rts_file =
"${root_build_dir}/gen/rts/${invoker.target_name}_inverted.filter"
args = [
rebase_path(rts_file, root_build_dir),
rebase_path(inverted_rts_file, root_build_dir),
]
outputs = [
rts_file,
inverted_rts_file,
]
args = [ rebase_path(rts_file, root_build_dir) ]
outputs = [ rts_file ]
}
}

Expand Down
63 changes: 24 additions & 39 deletions tools/mb/mb.py
Original file line number Diff line number Diff line change
Expand Up @@ -485,12 +485,10 @@ def RtsSelect(self):
exe += '.exe'

args = [
exe,
'select',
'-gen-inverse',
'-model-dir', model_dir, \
'-out', self.PathJoin(self.ToAbsPath(self.args.path), self.rts_out_dir),
'-checkout', self.chromium_src_dir,
exe, 'select',
'-model-dir', model_dir, \
'-out', self.PathJoin(self.ToAbsPath(self.args.path), self.rts_out_dir),
'-checkout', self.chromium_src_dir,
]
if self.args.rts_target_change_recall:
if (self.args.rts_target_change_recall < 0
Expand Down Expand Up @@ -1336,24 +1334,29 @@ def GenerateIsolates(self, vals, ninja_targets, isolate_map, build_dir):
if 'is_skylab=true' in vals['gn_args']:
runtime_deps = self._FilterOutUnneededSkylabDeps(runtime_deps)

# For more info about RTS, please see
# //docs/testing/regression-test-selection.md
if self.args.rts:
self.AddFilterFileArg(target, build_dir, command)

canonical_target = target.replace(':','_').replace('/','_')
ret = self.WriteIsolateFiles(build_dir, command, canonical_target,
runtime_deps, vals, extra_files)
if ret != 0:
return ret
return 0

def AddFilterFileArg(self, target, build_dir, command, inverted=False):
filter_file = ('%s_inverted' % target if inverted else target) + '.filter'
filter_file_path = self.PathJoin(self.rts_out_dir, filter_file)
abs_filter_file_path = self.ToAbsPath(build_dir, filter_file_path)

filter_exists = self.Exists(abs_filter_file_path)
if filter_exists:
command.append('--test-launcher-filter-file=%s' % filter_file_path)
self.Print('added RTS filter file to command: %s' % filter_file)
def AddFilterFileArg(self, target, build_dir, command):
if target in self.banned_from_rts:
self.Print('%s is banned for RTS on this builder' % target)
else:
filter_file = target + '.filter'
filter_file_path = self.PathJoin(self.rts_out_dir, filter_file)
abs_filter_file_path = self.ToAbsPath(build_dir, filter_file_path)

return filter_exists
if self.Exists(abs_filter_file_path):
command.append('--test-launcher-filter-file=%s' % filter_file_path)
self.Print('added RTS filter file to command: %s' % filter_file)

def PossibleRuntimeDepsPaths(self, vals, ninja_targets, isolate_map):
"""Returns a map of targets to possible .runtime_deps paths.
Expand Down Expand Up @@ -1553,31 +1556,13 @@ def WriteIsolateFiles(self, build_dir, command, target, runtime_deps, vals,
'list files in directory instead for:' + err)
return 1

isolate = {
self.WriteFile(isolate_path,
json.dumps({
'variables': {
'command': command,
'files': files,
'command': command,
'files': files,
}
}
# For more info about RTS, please see
# //docs/testing/regression-test-selection.md
if self.args.rts:
if target in self.banned_from_rts:
self.Print('%s is banned for RTS on this builder' % target)
isolate['variables']['command'] = command
else:
inverted_command = command.copy()
self.AddFilterFileArg(target, build_dir, command, inverted=False)
isolate['variables']['command'] = command

inverted_filter_exists = self.AddFilterFileArg(target,
build_dir,
inverted_command,
inverted=True)
if inverted_filter_exists:
isolate['variables']['inverted_command'] = inverted_command

self.WriteFile(isolate_path, json.dumps(isolate, sort_keys=True) + '\n')
}, sort_keys=True) + '\n')

self.WriteJSON(
{
Expand Down

0 comments on commit aa30199

Please sign in to comment.