WebRTC Android APK trybot recipe

scripts/slave/recipe_modules/webrtc/api.py

+# Copyright 2013 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.
+
+from slave import recipe_api
+
+class WebRTCApi(recipe_api.RecipeApi):
+  def __init__(self, **kwargs):
+    super(WebRTCApi, self).__init__(**kwargs)
+    self._env = {}
+
+  def get_config_defaults(self):
+    return {
+      'ANDROID_APK': False,
+    }
+
+  def get_normal_tests(self):
+    return [
+        'audio_decoder_unittests',
+        'common_audio_unittests',
+        'common_video_unittests',
+        'libjingle_media_unittest',
+        'libjingle_p2p_unittest',
+        'libjingle_peerconnection_unittest',
+        'libjingle_sound_unittest',
+        'libjingle_unittest',
+        'metrics_unittests',
+        'modules_tests',
+        'modules_unittests',
+        'neteq_unittests',
+        'system_wrappers_unittests',
+        'test_support_unittests',
+        'tools_unittests',
+        'video_engine_core_unittests',
+        'video_engine_tests',
+        'voice_engine_unittests',
+    ]

[iannucci, 2013/11/18 18:49:26]

I would even just make this a class attribute NORMAL_TESTS

api.webrtc.NORMAL_TESTS

[kjellander_chromium, 2013/11/20 14:29:16]

Right, I don't remember why I put it into a method in the first place. I think I
used to pass a parameter or something...

+
+  def envsetup(self):
+    envsetup_cmd = [self.m.path.checkout('build', 'android', 'envsetup.sh')]
+    # TODO(kjellander): Add support for --target-arch flag to support
+    # non-ARM architecture.
+    cmd = ([self.m.path.build('scripts', 'slave', 'env_dump.py'),
+            '--output-json', self.m.json.output()] + envsetup_cmd)
+    yield self.m.step('envsetup', cmd, env=self._env)

[iannucci, 2013/11/18 18:49:26]

envsetup makes me sad panda :(

It would be super cool if we could just call the NDK's envsetup directly, but
that's probably wishful thinking.

[kjellander_chromium, 2013/11/20 14:29:16]

I don't know the details about this. My primary target was to reproduce what's
currently running in our build waterfall for this trybot role.

+
+    env_diff = self.m.step_history.last_step().json.output
+    for key, value in env_diff.iteritems():
+      self._env[key] = value

[iannucci, 2013/11/18 18:49:26]

wdyt about putting this envsetup stuff in a 'base_android' module which can
contain all the sane/non-controversial android support tools? Then as we
discover more similarities/refactor craziness we can put them into base_android
until it's the only one left.

[kjellander_chromium, 2013/11/20 14:29:16]

Yes, I'm doing that in the new patch set.

+
+  def runhooks(self, **kwargs):
+    kwargs['env'] = self._env

[iannucci, 2013/11/18 18:49:26]

What do you think about 

kwargs.setdefault('env', self._env) 

or

old = kwargs.get('env', {})
kwargs['env'] = self._env.copy()
kwargs['env'].update(old)

?

I would at least add an

assert 'env' not in kwargs

So then callers don't get confused when they try to add env=... to the call.

[kjellander_chromium, 2013/11/20 14:29:16]

For now, I'll rather skip kwargs and let someone add it when it's needed (if
ever).
That makes the code cleaner (I've done so in the base_android/api.py)

+    return self.m.chromium.runhooks(**kwargs)
+
+  def compile(self, **kwargs):
+    kwargs['env'] = self._env
+    return self.m.chromium.compile(**kwargs)
+
+  def apply_svn_patch(self):
+    script = self.m.path.build('scripts', 'slave', 'apply_svn_patch.py')
+    args = ['-p', self.m.properties['patch_url'],
+            '-r', self.c.patch_root_dir]
+
+    # Allow manipulating patches for try jobs.
+    if self.c.patch_path_filter_script and self.c.patch_path_filter:
+      args += ['--path-filter', self.c.patch_path_filter,
+               '--path-filter-script', self.c.patch_path_filter_script,
+               '--strip-level', self.c.patch_strip_level]
+    return self.m.python('apply_patch', script, args)
+
+  def runtests(self, test, **kwargs):

[iannucci, 2013/11/18 18:49:26]

This should probably be test_runner. ('runtests' instead of 'runtest' was a typo
I need to fix).

As much as possible, we try to keep the names of the functions matching the
names of the scripts that they call. Then when someone's thinking about
test_runner, they are more confident that the 'test_runner' method is what they
want.

Later when we consolidate all the testing scripts, we can pick a standard global
name, but for now there are so many different scripts which do slightly
different things that it's silly.

[kjellander_chromium, 2013/11/20 14:29:16]

OK. I renamed it in the base_android/api.py

+    script = self.m.path.checkout('build', 'android', 'test_runner.py')
+    args = ['gtest', '-s', test, '--verbose']
+    if self.m.chromium.c.BUILD_CONFIG == 'Release':
+      args += ['--release']
+    return self.m.python(test, script, args, env=self._env)