Add recipe for split AMP/local CQ.

scripts/slave/recipe_modules/chromium/steps.py

 # Copyright 2014 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.
 
 import re
 import string
 
 
 class Test(object):
   """
@@ skipping 266 matching lines @@
       kwargs['flakiness_dashboard'] = 'http://test-results.appspot.com'
     else:
       kwargs['xvfb'] = True
       kwargs['test_type'] = self.name
       kwargs['annotate'] = 'gtest'
       kwargs['test_launcher_summary_output'] = gtest_results_file
       kwargs.update(self._runtest_kwargs)
 
     try:
       if is_android:
         api.chromium_android.run_test_suite(self.target_name, **kwargs)

[jbudorick, 2015/05/07 20:31:46]

chromium.steps already uses chromium_android, which also depends on chromium.
I'm not quite sure how -- chromium_android isn't in chromium.__init__ either --
but maybe we can emulate that?

[navabi, 2015/05/18 15:51:48]

Thanks. I'll take a look and see if I can figure it out.

       elif self._use_isolate:
         api.isolate.runtest(self.target_name, self._revision,
                             self._webkit_revision, **kwargs)
       else:
         api.chromium.runtest(self.target_name, revision=self._revision,
                              webkit_revision=self._webkit_revision, **kwargs)
     finally:
       step_result = api.step.active_result
       self._test_runs[suffix] = step_result
 
@@ skipping 313 matching lines @@
     if not gtest_results:
       return False, None  # pragma: no cover
 
     global_tags = gtest_results.raw.get('global_tags', [])
     if 'UNRELIABLE_RESULTS' in global_tags:
       return False, None  # pragma: no cover
 
     return True, gtest_results.failures
 
 
+class AMPGTestTest(Test):
+  AMP_INSTANCE_ADDRESS = '172.22.21.180'
+  AMP_INSTANCE_PORT = '80'
+  AMP_INSTANCE_PROTOCOL = 'http'
+  AMP_RESULTS_BUCKET = 'chrome-amp-results'
+  def __init__(self, name, args=None, target_name=None,
+               android_isolate_path=None, **runtest_kwargs):
+    self._name = name
+    self._args = args
+    self._target_name = target_name
+    self._android_isolate_path = android_isolate_path
+    # LocalGTestTest is used when AMP tests are not triggered successfully.
+    self._local_test = LocalGTestTest(name, args, target_name, **runtest_kwargs)
+
+  @property
+  def name(self):
+    return self._name
+
+  @property
+  def target_name(self):
+    return self._local_test.target_name
+
+  @property
+  def isolate_target(self):
+    return self._local_test.isolate_target
+
+  def compile_targets(self, api):
+    return self._local_test.compile_targets(api)
+
+  def run(self, api, suffix):  # pylint: disable=R0201
+    """Not used. All logic in pre_run, post_run."""
+    return []
+
+  def pre_run(self, api, suffix):
+    """Triggers an AMP test."""
+    amp_arguments = api.amp.amp_arguments(

[navabi, 2015/05/07 20:16:49]

this is one reason i need the amp module here.

+        api_address=AMPGTestTest.AMP_INSTANCE_ADDRESS,
+        api_port=AMPGTestTest.AMP_INSTANCE_PORT,
+        api_protocol=AMPGTestTest.AMP_INSTANCE_PROTOCOL,
+        device_minimum_os=builder.get('device_minimum_os'),
+        device_name=builder.get('device_name'),
+        device_os=builder.get('device_os'),
+        device_timeout=builder.get('device_timeout'))
+
+    isolate_file_path = (api.path['checkout'].join(self._android_isolate_path)
+                         if self._android_isolate_path else None)
+    deferred_trigger_result = api.amp.trigger_test_suite(
+        self._name, 'gtest',
+        api.amp.gtest_arguments(self._name,
+                                isolate_file_path=isolate_file_path),
+        amp_arguments)
+    self._trigger_successful = deferred_trigger_result.is_ok
+
+  def post_run(self, api, suffix):
+    # If we were unable to successfully trigger the AMP job, run locally.

[Paweł Hajdan Jr., 2015/05/08 08:55:11]

Why do we need this fallback logic? We don't do this anywhere else.

If it's really needed, why not perform the local run in the "run" method as
opposed to "post_run"? That'd be more consistent with how other tests are
running.

[navabi, 2015/05/18 15:51:49]

It is needed. We need this fallback logic because AMP may not have devices
available. When that is the case, the AMP tests, like the non-AMP tests, will
need to run locally. This will allow us to use AMP without requiring their lab
to immediately have full capacity.

The comment was meant to explain this. Let me know if you want more details in
the comment.
We only want to run locally if we have triggered the AMP job, waiting for
devices to become available, done everything else we need to do (i.e. completed
the non-AMP tests, and find that the AMP tests were unable to run. In this case,
we run locally?

Are you suggesting moving the return statement on line 677 into the local run
method and then on line 677 just calling the local run method?

[Paweł Hajdan Jr., 2015/06/08 09:33:21]

I think it'd be helpful to include what you said above in the comment.
Yes, I think it'd be better to move self._local_test.run call (guarded by
self._trigger_successful) to the run method.

[navabi, 2015/06/18 19:47:48]

Done and done.

[navabi, 2015/06/18 21:20:58]

Actually done and undone. We can't have the call to local test in the classes
run method. The reason is because the recipe does the following: pre_run, then
run, then post_run. That's why swarming tests do nothing for their run method.
It is not until post_run, when we determine the trigger failed that we want to
run tests locally.

We could always put something in the run method that says, if we haven't tried
to trigger, don't run, and if we did try to trigger but failed do run. But that
becomes much less clear as to what is going on.

[Paweł Hajdan Jr., 2015/06/19 10:46:09]

That's correct, the order of calls is like you said above. I don't see how that
prevents running the local fallback in the run method. Could you explain more?

It seems to me run could do "if not self._trigger_successful: run local
fallback", and post_run could do "if self._trigger_successful: collect AMP
results". Is there something that would break with that logic?
I'm confused by above statement. swarming doesn't do local fallback, and so it
always collects results asynchronously in post_run; there's nothing that could
possibly be done in the run method for swarming.
Could you explain more why? self._trigger_successful is set in pre_run, so it
should be available in both run and post_run.
I'm indeed confused by the way above logic is phrased. Is it the same though as
saying (in run): if trigger failed, run the local fallback? That sounds simple
and clear to me.

+    if not self._trigger_successful:
+      return self._local_test.run(api, suffix)
+    else:
+      amp_arguments = api.amp.amp_arguments(
+          api_address=AMPGTestTest.AMP_INSTANCE_ADDRESS,
+          api_port=AMPGTestTest.AMP_INSTANCE_PORT,
+          api_protocol=AMPGTestTest.AMP_INSTANCE_PROTOCOL,
+          device_minimum_os=builder.get('device_minimum_os'),
+          device_name=builder.get('device_name'),
+          device_os=builder.get('device_os'),
+          device_timeout=builder.get('device_timeout'))
+
+      deferred_step_result = api.amp.collect_test_suite(
+          self._name, 'gtest', api.amp.gtest_arguments(self._name),
+          amp_arguments)
+      self._test_runs[suffix] = deferred_step_result
+      return deferred_step_result
+
+  def has_valid_results(self, api, suffix):
+    return self._local_test.has_valid_results(api, suffix)
+
+  def failures(self, api, suffix):
+    return self._local_test.failures(api, suffix)
+
+
 class GTestTest(Test):
   def __init__(self, name, args=None, target_name=None, enable_swarming=False,
                swarming_shards=1, swarming_dimensions=None, swarming_tags=None,
-               swarming_extra_suffix=None, **runtest_kwargs):
+               swarming_extra_suffix=None, amp_test=False, **runtest_kwargs):
     super(GTestTest, self).__init__()
     if enable_swarming:
       self._test = SwarmingGTestTest(
           name, args, target_name, swarming_shards, swarming_dimensions,
           swarming_tags, swarming_extra_suffix)
+    elif amp_test:

[Paweł Hajdan Jr., 2015/05/08 08:55:11]

Do we need to modify GTestTest? Why wouldn't callers just use AMPGTestTest
directly?

[navabi, 2015/05/18 15:51:49]

This was to make it like SwarmingGTestTest. Notice how the callers dont just use
SwarmingGTestTest directly. In fact, my original patch used the AMPGTestTest
directly, but in this patch I made it more like the swarming tests.

[navabi, 2015/06/18 19:47:48]

Done. Now the callers use AMPGTestTest directly.

+      self._test = AMPGTestTest(name, args, target_name, **runtest_kwargs)
     else:
       self._test = LocalGTestTest(name, args, target_name, **runtest_kwargs)
 
   @property
   def name(self):
     return self._test.name
 
   @property
   def isolate_target(self):
     return self._test.isolate_target
@@ skipping 608 matching lines @@
   GTestTest('ui_base_unittests'),
   GTestTest('ui_ios_unittests'),
   GTestTest('sync_unit_tests'),
   GTestTest('sql_unittests'),
 ]
 
 GOMA_TESTS = [
   GTestTest('base_unittests'),
   GTestTest('content_unittests'),
 ]