[ios] have MB check mb_config.pyl first, before checking iOS

As we migrate iOS bots to the Chromium recipe, we want the mb
configurations defined under mb_config.pyl to have precedence
over the previous JSON definitions.

There are two checks that trigger the iOS search: 1) when the
config doesn't exist, and 2) when the config is ios_error.

These additions are to be removed once the migration is complete.

Bug: 912681
Change-Id: Ieb04e98724460c2aba3bfd94d37b855f518ba7e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2106562
Commit-Queue: Jeff Yoon <jeffyoon@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751213}

tools/mb/mb.py

@@ -866,23 +866,45 @@ class MetaBuildWrapper(object):
     return ' '.join(gn_args)
 
   def Lookup(self):
-    vals = self.ReadIOSBotConfig()
-    if not vals:
-      self.ReadConfigFile()
+    self.ReadConfigFile()
+    try:
       if self.group_by_bucket:
         config = self.ConfigFromArgsBucket()
       else:
         config = self.ConfigFromArgs()
-      if config.startswith('//'):
-        if not self.Exists(self.ToAbsPath(config)):
-          raise MBErr('args file "%s" not found' % config)
-        vals = DefaultVals()
-        vals['args_file'] = config
-      else:
-        if not config in self.configs:
-          raise MBErr('Config "%s" not found in %s' %
-                      (config, self.args.config_file))
-        vals = FlattenConfig(self.configs, self.mixins, config)
+    except MBErr as e:
+      # TODO(crbug.com/912681) While iOS bots are migrated to use the
+      # Chromium recipe, we want to ensure that we're checking MB's
+      # configurations first before going to iOS.
+      # This is to be removed once the migration is complete.
+      vals = self.ReadIOSBotConfig()
+      if not vals:
+        raise e
+      return vals
+
+    # TODO(crbug.com/912681) Some iOS bots have a definition, with ios_error
+    # as an indicator that it's incorrect. We utilize this to check the
+    # iOS JSON instead, and error out if there exists no definition at all.
+    # This is to be removed once the migration is complete.
+    if config == 'ios_error':
+      vals = self.ReadIOSBotConfig()
+      if not vals:
+        raise MBErr('No iOS definition was found. Please ensure there is a '
+                    'definition for the given iOS bot under '
+                    'mb_config.pyl or a JSON file definition under '
+                    '//ios/build/bots.')
+      return vals
+
+    if config.startswith('//'):
+      if not self.Exists(self.ToAbsPath(config)):
+        raise MBErr('args file "%s" not found' % config)
+      vals = DefaultVals()
+      vals['args_file'] = config
+    else:
+      if not config in self.configs:
+        raise MBErr(
+            'Config "%s" not found in %s' % (config, self.args.config_file))
+      vals = FlattenConfig(self.configs, self.mixins, config)
     return vals
 
   def ReadIOSBotConfig(self):

tools/mb/mb_unittest.py

@@ -142,6 +142,7 @@ TEST_CONFIG = """\
       'fake_args_bot': '//build/args/bots/fake_master/fake_args_bot.gn',
       'fake_multi_phase': { 'phase_1': 'phase_1', 'phase_2': 'phase_2'},
       'fake_args_file': 'args_file_goma',
+      'fake_ios_error': 'ios_error',
     },
   },
   'configs': {
@@ -151,11 +152,15 @@ TEST_CONFIG = """\
     'debug_goma': ['debug', 'goma'],
     'phase_1': ['phase_1'],
     'phase_2': ['phase_2'],
+    'ios_error': ['error'],
   },
   'mixins': {
     'cros_chrome_sdk': {
       'cros_passthrough': True,
     },
+    'error': {
+      'gn_args': 'error',
+    },
     'fake_feature1': {
       'gn_args': 'enable_doom_melon=true',
     },
@@ -422,7 +427,6 @@ class UnitTest(unittest.TestCase):
       actual_ret = mbw.Main(args)
     finally:
       os.environ = prev_env
-
     self.assertEqual(actual_ret, ret)
     if out is not None:
       self.assertEqual(mbw.out, out)
@@ -1010,6 +1014,67 @@ class UnitTest(unittest.TestCase):
     self.assertIn(['autoninja.bat', '-C', 'out\\Default', 'base_unittests'],
                   mbw.calls)
 
+  def test_ios_error_config_with_ios_json(self):
+    """Ensures that ios_error config finds the correct iOS JSON file for args"""
+    files = {
+        '/fake_src/ios/build/bots/fake_master/fake_ios_error.json':
+        ('{"gn_args": ["is_debug=true"]}\n')
+    }
+    mbw = self.fake_mbw(files)
+    self.check(['lookup', '-m', 'fake_master', '-b', 'fake_ios_error'],
+               mbw=mbw,
+               ret=0,
+               out=('\n'
+                    'Writing """\\\n'
+                    'is_debug = true\n'
+                    '""" to _path_/args.gn.\n\n'
+                    '/fake_src/buildtools/linux64/gn gen _path_\n'))
+
+  def test_bot_definition_in_ios_json_only(self):
+    """Ensures that logic checks iOS JSON file for args
+
+    When builder definition is not present, ensure that ios/build/bots/ is
+    checked.
+    """
+    files = {
+        '/fake_src/ios/build/bots/fake_master/fake_ios_bot.json':
+        ('{"gn_args": ["is_debug=true"]}\n')
+    }
+    mbw = self.fake_mbw(files)
+    self.check(['lookup', '-m', 'fake_master', '-b', 'fake_ios_bot'],
+               mbw=mbw,
+               ret=0,
+               out=('\n'
+                    'Writing """\\\n'
+                    'is_debug = true\n'
+                    '""" to _path_/args.gn.\n\n'
+                    '/fake_src/buildtools/linux64/gn gen _path_\n'))
+
+  def test_ios_error_config_missing_json_definition(self):
+    """Ensures MBErr is thrown
+
+    Expect MBErr with 'No iOS definition ...' for iOS bots when the bot config
+    is ios_error, but there is no iOS JSON definition for it.
+    """
+    mbw = self.fake_mbw()
+    self.check(['lookup', '-m', 'fake_master', '-b', 'fake_ios_error'],
+               mbw=mbw,
+               ret=1)
+    self.assertIn('MBErr: No iOS definition was found.', mbw.out)
+
+  def test_bot_missing_definition(self):
+    """Ensures builder missing MBErr is thrown
+
+    Expect the original MBErr to be thrown for iOS bots when the bot definition
+    doesn't exist at all.
+    """
+    mbw = self.fake_mbw()
+    self.check(['lookup', '-m', 'fake_master', '-b', 'random_bot'],
+               mbw=mbw,
+               ret=1)
+    self.assertIn('MBErr: Builder name "random_bot"  not found under masters',
+                  mbw.out)
+
 
 if __name__ == '__main__':
   unittest.main()