Revert "Reland "trigger_scripts: support go client in chromeos_device_trigger""

This reverts commit 487e7412.

Reason for revert:
https://ci.chromium.org/p/chrome/builders/ci/Win%207%20Nvidia%20GPU%20Perf/17285

Original change's description:
> Reland "trigger_scripts: support go client in chromeos_device_trigger"
>
> This reverts commit 78d95924.
>
> Reason for revert:
> This fixes previous failure and has a test
>
> Original change's description:
> > Revert "trigger_scripts: support go client in chromeos_device_trigger"
> >
> > This reverts commit 3fd9ba25.
> >
> > Reason for revert:
> > https://ci.chromium.org/p/chrome/builders/ci/android-go_webview-perf/23071
> >
> > Original change's description:
> > > trigger_scripts: support go client in chromeos_device_trigger
> > >
> > > This scripts needs json conversion similar to
> > > https://source.chromium.org/chromium/chromium/src/+/master:testing/trigger_scripts/base_test_triggerer.py;l=316-334;drc=e4b6e9349b783a53e3efcc45e54e6632f874ca62
> > > when using go client.
> > >
> > > This is fix for breakage like
> > > https://ci.chromium.org/p/chrome/builders/ci/chromeos-eve-chrome/12159
> > >
> > > Cq-Include-Trybots: luci.chromium.try:chromeos-kevin-rel
> > > Cq-Include-Trybots: luci.chrome.try:chromeos-eve-chrome
> > > Bug: 1127205
> > > Change-Id: Ibf980d44b4b30855ed61e3d487bd683adf948003
> > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2635275
> > > Reviewed-by: Ben Pastene <bpastene@chromium.org>
> > > Reviewed-by: Wenbin Zhang <wenbinzhang@google.com>
> > > Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
> > > Auto-Submit: Takuto Ikuta <tikuta@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#844979}
> >
> > TBR=tikuta@chromium.org,bpastene@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com,wenbinzhang@google.com
> >
> > Change-Id: I96bf20546c714a40a53c44afb57bab5924da5d7e
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug: 1127205
> > Cq-Include-Trybots: luci.chromium.try:chromeos-kevin-rel
> > Cq-Include-Trybots: luci.chrome.try:chromeos-eve-chrome
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2638954
> > Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
> > Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#844987}
>
> # Not skipping CQ checks because this is a reland.
>
> Bug: 1127205
> Change-Id: I6b408eed0dbf16608b6ee7084ec093ff4ab51233
> Cq-Include-Trybots: luci.chrome.try:chromeos-eve-chrome
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2639441
> Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
> Auto-Submit: Takuto Ikuta <tikuta@chromium.org>
> Reviewed-by: Brian Sheedy <bsheedy@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#845499}

TBR=kbr@chromium.org,tikuta@chromium.org,bsheedy@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com,wenbinzhang@google.com

Change-Id: I94525d422ea33245c3142696752cbf64f02e841a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1127205
Cq-Include-Trybots: luci.chrome.try:chromeos-eve-chrome
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2639349Reviewed-by: Takuto Ikuta <tikuta@chromium.org>
Commit-Queue: Takuto Ikuta <tikuta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845511}

testing/trigger_scripts/base_test_triggerer.py

@@ -239,31 +239,10 @@ class BaseTestTriggerer(object):
       logging.info('Running Swarming with args: %s', args)
     return subprocess.call([sys.executable, SWARMING_PY] + args)
 
-  def run_swarming_go(self, args, verbose, json_path, shard_index, shards,
-                      merged_json=None):
+  def run_swarming_go(self, args, verbose):
     if verbose:
       logging.info('Running Go `swarming` with args: %s', args)
-
-    ret = subprocess.call([SWARMING_GO] + _convert_to_go_swarming_args(args))
-    result_json = self.read_json_from_temp_file(json_path)
-    if not merged_json:
-      # Copy the entire JSON -- in particular, the "request"
-      # dictionary -- from the first shard. "swarming.py collect" uses
-      # some keys from this dictionary, in particular related to
-      # expiration. It also contains useful debugging information.
-      merged_json = copy.deepcopy(result_json)
-      # However, reset the "tasks" entry to an empty dictionary,
-      # which will be handled specially.
-      merged_json['tasks'] = {}
-    tasks = {
-      task['request']['task_id']: task['request']
-      for task  in result_json['tasks']
-    }
-    for k, v in tasks.items():
-      v['shard_index'] = shard_index
-      merged_json['tasks'][k + ':%d:%d' % (shard_index, shards)] = v
-    self.write_json_to_file(merged_json, json_path)
-    return ret
+    return subprocess.call([SWARMING_GO] + _convert_to_go_swarming_args(args))
 
   def prune_test_specific_configs(self, args, verbose):
     # Ability for base class to further prune configs to
@@ -328,19 +307,12 @@ class BaseTestTriggerer(object):
         args_to_pass = self.modify_args(filtered_remaining_args, bot_index,
                                         shard_index, args.shards, json_temp)
         if args.use_swarming_go:
-          ret = self.run_swarming_go(
-            args_to_pass, verbose, json_temp, shard_index, args.shards,
-            merged_json)
+          ret = self.run_swarming_go(args_to_pass, verbose)
         else:
           ret = self.run_swarming(args_to_pass, verbose)
         if ret:
           sys.stderr.write('Failed to trigger a task, aborting\n')
           return ret
-
-        if args.use_swarming_go:
-          continue
-
-        # TODO(crbug.com/1127205): remove belows in this block.
         result_json = self.read_json_from_temp_file(json_temp)
         if not merged_json:
           # Copy the entire JSON -- in particular, the "request"
@@ -352,6 +324,11 @@ class BaseTestTriggerer(object):
           # which will be handled specially.
           merged_json['tasks'] = {}
         tasks = result_json['tasks']
+        if args.use_swarming_go:
+          # TODO(crbug.com/1127205): remove this
+          tasks = {
+            task['request']['task_id']: task['request'] for task in tasks
+          }
         for k, v in tasks.items():
           v['shard_index'] = shard_index
           merged_json['tasks'][k + ':%d:%d' % (shard_index, args.shards)] = v

testing/trigger_scripts/base_test_triggerer_unittest.py

-#!/usr/bin/env vpython
 # Copyright 2020 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.
@@ -6,20 +5,12 @@
 """Tests for base_device_trigger.py."""
 
 import argparse
-import json
 import unittest
 
-import mock
-
-from pyfakefs import fake_filesystem_unittest
-
 import base_test_triggerer
 
 
-class UnitTest(fake_filesystem_unittest.TestCase):
-  def setUp(self):
-    self.setUpPyfakefs()
-
+class UnitTest(unittest.TestCase):
   def test_convert_to_go_swarming_args(self):
     args = [
         '--swarming', 'x.apphost.com', '--dimension', 'pool', 'ci',
@@ -68,36 +59,6 @@ class UnitTest(fake_filesystem_unittest.TestCase):
       self.assertRaises(ex, base_test_triggerer._convert_to_go_swarming_args,
                         args)
 
-  def test_run_swarming_go(self):
-    triggerer = base_test_triggerer.BaseTestTriggerer()
-
-    dump_json = 'dump_json'
-    with open(dump_json, 'w') as f:
-      f.write(json.dumps({
-        'tasks': [{
-          'request': {
-            'task_id': 'f0',
-          },
-        }]
-      }))
-
-    with mock.patch('subprocess.call', return_value=0) as mock_call:
-      triggerer.run_swarming_go([], False, dump_json, 0, 1, {})
-      mock_call.assert_called_once()
-
-    with open(dump_json) as f:
-      self.assertEqual(
-        json.load(f),
-        {
-          u'tasks': {
-            u'f0:0:1': {
-              u'shard_index': 0,
-              u'task_id': u'f0'
-            }
-          }
-        })
-
-
   def test_arg_parser(self):
     # Added for https://crbug.com/1143224
     parser = argparse.ArgumentParser()

testing/trigger_scripts/chromeos_device_trigger.py

@@ -65,10 +65,18 @@ def parse_args(triggerer):
       dest='optional_dimensions',
       help='Optional dimensions which will result in additional task slices. '
            'Duplicated from the `swarming.py trigger` command.')
-  base_test_triggerer.BaseTestTriggerer.setup_parser_contract(parser)
-  args, additional_args = parser.parse_known_args()
-  additional_args = triggerer.modify_args(
-      additional_args, 0, args.shard_index, args.shards, args.dump_json)
+  # BaseTestTriggerer's setup_parser_contract() takes care of adding needed
+  # swarming.py args if they're not already present. But only do this if
+  # '--shard-index' is passed in. (The exact usage of trigger scripts are
+  # currently changing. See crbug.com/926987 for more info.)
+  if '--shard-index' in sys.argv:
+    base_test_triggerer.BaseTestTriggerer.setup_parser_contract(parser)
+    args, additional_args = parser.parse_known_args()
+    additional_args = triggerer.modify_args(
+        additional_args, 0, args.shard_index, args.shards, args.dump_json)
+  else:
+    base_test_triggerer.BaseTestTriggerer.add_use_swarming_go_arg(parser)
+    args, additional_args = parser.parse_known_args()
 
   if additional_args[0] != 'trigger':
     parser.error(
@@ -122,9 +130,7 @@ def main():
   new_args += additional_args[1:]
 
   if args.use_swarming_go:
-    return triggerer.run_swarming_go(
-      new_args, True, args.dump_json, args.shard_index, args.shards)
-
+    return triggerer.run_swarming_go(new_args, True)
   return triggerer.run_swarming(new_args, True)
 
 

testing/trigger_scripts/perf_device_trigger_unittest.py

@@ -53,8 +53,7 @@ class FakeTriggerer(perf_device_trigger.PerfDeviceTriggerer):
     del verbose #unused
     self._swarming_runs.append(args)
 
-  def run_swarming_go(self, args, verbose, _json_path, _shard_index, _shard,
-                      _merged_json=None):
+  def run_swarming_go(self, args, verbose):
     self._triggered_with_swarming_go += 1
     self.run_swarming(args, verbose)