Revert of Update MB to use `gn analyze`. (patchset #9 id:160001 of...

Revert of Update MB to use `gn analyze`. (patchset #9 id:160001 of https://codereview.chromium.org/2298403002/ )

Reason for revert:
Need to add "aura_builder", probably others ...

Original issue's description:
> Update MB to use `gn analyze`.
>
> Now that GN supports a form of `analyze` directly, this patch
> updates //tools/mb to use it rather than working awkwardly through
> `gn refs`. The MB implementation now is mostly responsible for
> mapping ninja targets to labels, calling GN, and then mapping returned
> labels back to ninja targets. Eventually we should update the
> recipes to just pass labels around instead.
>
> One significant change is that now we need entries in gn_isolate_map.pyl
> for every target listed in 'additional_compile_targets' and
> 'isolated_scripts' as well as in 'gtest_tests', because we have to remap
> everything to the GN labels, not just tests for swarming.
>
> We should switch everything to using GN labels directly, but that'll require
> follow-on CLs.
>
> R=brettw@chromium.org
> BUG=555273
>
> Committed: https://crrev.com/9aba8b21177df3ed3e897d5c500d845fbf5523e6
> Cr-Commit-Position: refs/heads/master@{#419315}

TBR=phajdan.jr@chromium.org,brettw@chromium.org,kbr@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=555273

Review-Url: https://codereview.chromium.org/2341403004
Cr-Commit-Position: refs/heads/master@{#419321}

testing/buildbot/gn_isolate_map.pyl

@@ -27,9 +27,6 @@
 #  "gpu_browser_test"
 #  : the test is a subset of the browser_tests that will be run against
 #    a real GPU.
-#  "additional_compile_target"
-#  : this isn't actually a test, but we still need a mapping from the 
-#    ninja target to the GN label in order to analyze it.
 #  "raw"
 #  : the test is a standalone executable; it may take an optional list of
 #    command line arguments in the "args" field, but otherwise needs no
@@ -64,10 +61,6 @@
 # "//testing/scripts/foo.py".
 
 {
-  "All": {
-    "label": "//:All",
-    "type": "additional_compile_target",
-  },
   "accessibility_unittests":  {
     "label": "//ui/accessibility:accessibility_unittests",
     "type": "raw",
@@ -198,18 +191,10 @@
     "label": "//chromecast/crash:cast_crash_unittests",
     "type": "console_test_launcher",
   },
-  "cast_media_unittests": {
-    "label": "//chromecast/media:cast_media_unittests",
-    "type": "console_test_launcher",
-  },
   "cast_shell_unittests": {
     "label": "//chromecast/app:cast_shell_unittests",
     "type": "console_test_launcher",
   },
-  "cast_shell_browser_test": {
-    "label": "//chromecast/browser:cast_shell_browser_test",
-    "type": "console_test_launcher",
-  },
   "cast_unittests": {
     "label": "//media/cast:cast_unittests",
     "type": "windowed_test_launcher",
@@ -218,10 +203,6 @@
     "label": "//cc:cc_unittests",
     "type": "windowed_test_launcher",
   },
-  "chrome": {
-    "label": "//chrome",
-    "type": "additional_compile_target",
-  },
   "chrome_app_unittests": {
     "label": "//chrome/test:chrome_app_unittests",
     "type": "console_test_launcher",
@@ -230,22 +211,10 @@
     "label": "//chrome_elf:chrome_elf_unittests",
     "type": "raw",
   },
-  "chrome_official_builder": {
-    "label": "//:chrome_official_builder",
-    "type": "additional_compile_target",
-  },
-  "chrome_public_apk": {
-    "label": "//chrome/android:chrome_public_apk",
-    "type": "additional_compile_target",
-  },
   "chrome_public_test_apk": {
     "label": "//chrome/android:chrome_public_test_apk",
     "type": "console_test_launcher",
   },
-  "chromium_swarm_tests": {
-    "label": "//:chromium_swarm_tests",
-    "type": "additional_compile_target",
-  },
   "chrome_sync_shell_test_apk": {
     "label": "//chrome/android:chrome_sync_shell_test_apk",
     "type": "console_test_launcher",
@@ -305,14 +274,6 @@
     "label": "//courgette:courgette_unittests",
     "type": "console_test_launcher",
   },
-  "cronet_package": {
-    "label": "//components/cronet/android:cronet_package",
-    "type": "additional_compile_target",
-  },
-  "cronet_test_instrumentation_apk": {
-    "label": "//cronet_test_instrumentation_apk",
-    "type": "additional_compile_target",
-  },
   "crypto_unittests": {
     "label": "//crypto:crypto_unittests",
     "type": "console_test_launcher",
@@ -380,10 +341,6 @@
     "type": "raw",
     "args": [],
   },
-  "gn_all": {
-    "label": "//:gn_all",
-    "type": "additional_compile_target",
-  },
   "gn_unittests": {
     "label": "//tools/gn:gn_unittests",
     "type": "raw",
@@ -401,22 +358,10 @@
     "label": "//gpu:gpu_unittests",
     "type": "windowed_test_launcher",
   },
-  "headless_lib": {
-    "label": "//headless:headless_lib",
-    "type": "additional_compile_target",
-  },
   "headless_browsertests": {
     "label": "//headless:headless_browsertests",
     "type": "console_test_launcher",
   },
-  "headless_shell": {
-    "label": "//headless:headless_shell",
-    "type": "additional_compile_target",
-  },
-  "headless_tests": {
-    "label": "//headless:headless_tests",
-    "type": "additional_compile_target",
-  },
   "headless_unittests": {
     "label": "//headless:headless_unittests",
     "type": "console_test_launcher",
@@ -454,23 +399,19 @@
     "type": "console_test_launcher",
   },
   # See http://crbug.com/585151
-  "libaddressinput_unittests": {
-    "label": "//third_party/libaddressinput:libaddressinput_unittests",
-    "type": "console_test_launcher",
-  },
+  #"libaddressinput_unittests": {
+    #"label": "//third_party/libaddressinput:libaddressinput_unittests",
+    #"type": "console_test_launcher",
+  #},
   # See http://crbug.com/585151
-  "libphonenumber_unittests": {
-    "label": "//third_party/libphonenumber:libphonenumber_unittests",
-    "type": "console_test_launcher",
-  },
+  #"libphonenumber_unittests": {
+    #"label": "//third_party/libphonenumber:libphonenumber_unittests",
+    #"type": "console_test_launcher",
+  #},
   "mac_installer_unittests": {
     "label": "//chrome/installer/mac/app:mac_installer_unittests",
     "type": "console_test_launcher",
   },
-  "mash:all" : {
-    "label": "//mash:all",
-    "type": "additional_compile_target",
-  },
   "media_unittests": {
     "label": "//media:media_unittests",
     "type": "windowed_test_launcher",
@@ -658,10 +599,6 @@
     "label": "//chrome/test:sync_integration_tests",
     "type": "windowed_test_launcher",
   },
-  "system_webview_apk": {
-    "label": "//android_webview:system_webview_apk",
-    "type": "additional_compile_target",
-  },
   "system_wrappers_unittests": {
     "label": "//webrtc/system_wrappers:system_wrappers_unittests",
     "type": "console_test_launcher",

testing/buildbot/manage.py

@@ -132,14 +132,28 @@ SKIP = {
 
 
 SKIP_GN_ISOLATE_MAP_TARGETS = {
-  'all',
-  'chromium_swarm_tests',
+  # TODO(GYP): These targets have not been ported to GN yet.
+  'android_webview_unittests',
+  'angle_deqp_gles2_tests',
+  'angle_deqp_gles3_tests',
+  'cast_media_unittests',
+  'cast_shell_browser_test',
+  'chromevox_tests',
+  'nacl_helper_nonsfi_unittests',
+
+  # TODO(kbr): teach this script about isolated_scripts tests.
+  # crbug.com/620531
+  'telemetry_gpu_integration_test',
+  'telemetry_gpu_test',
+  'telemetry_gpu_unittests',
+  'telemetry_perf_tests',
+  'telemetry_perf_unittests',
+  'telemetry_unittests',
 
   # These tests are only run on WebRTC CI.
   'audio_decoder_unittests',
   'common_audio_unittests',
   'common_video_unittests',
-  'frame_analyzer',
   'modules_tests',
   'modules_unittests',
   'peerconnection_unittests',
@@ -219,36 +233,23 @@ def process_file(mode, test_name, tests_location, filepath, ninja_targets,
       continue
     if not isinstance(data, dict):
       raise Error('%s: %s is broken: %s' % (filename, builder, data))
-    if ('gtest_tests' not in data and
-        'isolated_scripts' not in data and
-        'additional_compile_targets' not in data):
+    if 'gtest_tests' not in data:
       continue
-
-    for target in data.get('additional_compile_targets', []):
-      if (target not in ninja_targets and
-          target not in SKIP_GN_ISOLATE_MAP_TARGETS):
-        raise Error('%s: %s / %s is not listed in gn_isolate_map.pyl' %
-                    (filename, builder, target))
-      elif target in ninja_targets:
-        ninja_targets_seen.add(target)
-
-    gtest_tests = data.get('gtest_tests', [])
-    if not isinstance(gtest_tests, list):
+    if not isinstance(data['gtest_tests'], list):
       raise Error(
-          '%s: %s is broken: %s' % (filename, builder, gtest_tests))
-    if not all(isinstance(g, dict) for g in gtest_tests):
+          '%s: %s is broken: %s' % (filename, builder, data['gtest_tests']))
+    if not all(isinstance(g, dict) for g in data['gtest_tests']):
       raise Error(
-          '%s: %s is broken: %s' % (filename, builder, gtest_tests))
+          '%s: %s is broken: %s' % (filename, builder, data['gtest_tests']))
 
     seen = set()
-    for d in gtest_tests:
-      test = d['test']
-      if (test not in ninja_targets and
-          test not in SKIP_GN_ISOLATE_MAP_TARGETS):
+    for d in data['gtest_tests']:
+      if (d['test'] not in ninja_targets and
+          d['test'] not in SKIP_GN_ISOLATE_MAP_TARGETS):
         raise Error('%s: %s / %s is not listed in gn_isolate_map.pyl.' %
-                    (filename, builder, test))
-      elif test in ninja_targets:
-        ninja_targets_seen.add(test)
+                    (filename, builder, d['test']))
+      elif d['test'] in ninja_targets:
+        ninja_targets_seen.add(d['test'])
 
       name = d.get('name', d['test'])
       if name in seen:
@@ -258,18 +259,8 @@ def process_file(mode, test_name, tests_location, filepath, ninja_targets,
       d.setdefault('swarming', {}).setdefault(
           'can_use_on_swarming_builders', False)
 
-    if gtest_tests:
     config[builder]['gtest_tests'] = sorted(
-          gtest_tests, key=lambda x: x['test'])
-
-    for d in data.get('isolated_scripts', []):
-      name = d['isolate_name']
-      if (name not in ninja_targets and
-          name not in SKIP_GN_ISOLATE_MAP_TARGETS):
-        raise Error('%s: %s / %s is not listed in gn_isolate_map.pyl.' %
-                    (filename, builder, name))
-      elif name in ninja_targets:
-        ninja_targets_seen.add(name)
+        data['gtest_tests'], key=lambda x: x['test'])
 
     # The trick here is that process_builder_remaining() is called before
     # process_builder_convert() so tests_location can be used to know how many

tools/mb/mb_unittest.py

@@ -215,15 +215,6 @@ class UnitTest(unittest.TestCase):
   def fake_mbw(self, files=None, win32=False):
     mbw = FakeMBW(win32=win32)
     mbw.files.setdefault(mbw.default_config, TEST_CONFIG)
-    mbw.files.setdefault(
-      mbw.ToAbsPath('//testing/buildbot/gn_isolate_map.pyl'),
-      '''{
-        "foo_unittests": {
-          "label": "//foo:foo_unittests",
-          "type": "console_test_launcher",
-          "args": [],
-        },
-      }''')
     mbw.files.setdefault(
         mbw.ToAbsPath('//build/args/bots/fake_master/fake_gn_args_bot.gn'),
         'is_debug = false\n')
@@ -277,29 +268,78 @@ class UnitTest(unittest.TestCase):
     self.assertEqual(mbw.files['/fake_src/out/Debug/mb_type'], 'gyp')
 
   def test_gn_analyze(self):
-    files = {'/tmp/in.json': '''{\
+    files = {'/tmp/in.json': """{\
                "files": ["foo/foo_unittest.cc"],
-               "test_targets": ["foo_unittests"],
-               "additional_compile_targets": ["all"]
-             }''',
-             '/tmp/out.json.gn': '''{\
-               "status": "Found dependency",
-               "compile_targets": ["//foo:foo_unittests"],
-               "test_targets": ["//foo:foo_unittests"]
-             }'''}
+               "test_targets": ["foo_unittests", "bar_unittests"],
+               "additional_compile_targets": []
+             }"""}
 
     mbw = self.fake_mbw(files)
-    mbw.Call = lambda cmd, env=None, buffer_output=True: (0, '', '')
+    mbw.Call = lambda cmd, env=None, buffer_output=True: (
+        0, 'out/Default/foo_unittests\n', '')
 
     self.check(['analyze', '-c', 'gn_debug_goma', '//out/Default',
                 '/tmp/in.json', '/tmp/out.json'], mbw=mbw, ret=0)
     out = json.loads(mbw.files['/tmp/out.json'])
     self.assertEqual(out, {
       'status': 'Found dependency',
-      'compile_targets': ['foo:foo_unittests'],
+      'compile_targets': ['foo_unittests'],
       'test_targets': ['foo_unittests']
     })
 
+  def test_gn_analyze_fails(self):
+    files = {'/tmp/in.json': """{\
+               "files": ["foo/foo_unittest.cc"],
+               "test_targets": ["foo_unittests", "bar_unittests"],
+               "additional_compile_targets": []
+             }"""}
+
+    mbw = self.fake_mbw(files)
+    mbw.Call = lambda cmd, env=None, buffer_output=True: (1, '', '')
+
+    self.check(['analyze', '-c', 'gn_debug_goma', '//out/Default',
+                '/tmp/in.json', '/tmp/out.json'], mbw=mbw, ret=1)
+
+  def test_gn_analyze_all(self):
+    files = {'/tmp/in.json': """{\
+               "files": ["foo/foo_unittest.cc"],
+               "test_targets": ["bar_unittests"],
+               "additional_compile_targets": ["all"]
+             }"""}
+    mbw = self.fake_mbw(files)
+    mbw.Call = lambda cmd, env=None, buffer_output=True: (
+        0, 'out/Default/foo_unittests\n', '')
+    self.check(['analyze', '-c', 'gn_debug_goma', '//out/Default',
+                '/tmp/in.json', '/tmp/out.json'], mbw=mbw, ret=0)
+    out = json.loads(mbw.files['/tmp/out.json'])
+    self.assertEqual(out, {
+      'status': 'Found dependency (all)',
+      'compile_targets': ['all', 'bar_unittests'],
+      'test_targets': ['bar_unittests'],
+    })
+
+  def test_gn_analyze_missing_file(self):
+    files = {'/tmp/in.json': """{\
+               "files": ["foo/foo_unittest.cc"],
+               "test_targets": ["bar_unittests"],
+               "additional_compile_targets": []
+             }"""}
+    mbw = self.fake_mbw(files)
+    mbw.cmds = [
+        (0, '', ''),
+        (1, 'The input matches no targets, configs, or files\n', ''),
+        (1, 'The input matches no targets, configs, or files\n', ''),
+    ]
+
+    self.check(['analyze', '-c', 'gn_debug_goma', '//out/Default',
+                '/tmp/in.json', '/tmp/out.json'], mbw=mbw, ret=0)
+    out = json.loads(mbw.files['/tmp/out.json'])
+    self.assertEqual(out, {
+      'status': 'No dependency',
+      'compile_targets': [],
+      'test_targets': [],
+    })
+
   def test_gn_gen(self):
     mbw = self.fake_mbw()
     self.check(['gen', '-c', 'gn_debug_goma', '//out/Default', '-g', '/goma'],