From 48f53ba0951a5b624c89dab4cf8d10b9e243efa4 Mon Sep 17 00:00:00 2001
From: Andrew Gallagher <agallagher@fb.com>
Date: Tue, 3 Jan 2012 19:05:55 -0800
Subject: [PATCH] Allow updating diff with results for new unit tests

Summary:
When using postponed unittests to make 'arc diff' faster, there
are some situations where it is difficult to know exactly how
many unittests will be run.  This is the case for many of our
C++ unittests, which we can't really know until we compile the
tests (which is slow, and probably isn't reasonable to be done
before posting the diff).  I suppose we could make sure we
explicitly which tests a C++ unittest will run in some way, but
this would require a lot of change to our backend test infra.
Also, it seems that this is a pretty general issue of not knowing
how many unittests will be run until they actually run.

This diff adds an optional "create" parameter to updateunitresults
which wil create a new unit tests result rather than updating an
existing one.  I am not sure if this really fits here or should
be its own method, but there is a lot of code re-use between them
so I consolidated.

Test Plan: updated a diff with a new unit test result

Reviewers: epriestley, jungejason

Reviewed By: epriestley

CC: aran, epriestley, andrewjcg, tuomaspelkonen

Differential Revision: https://secure.phabricator.com/D1352
---
 ..._differential_updateunitresults_Method.php | 23 +++++++++++--------
 .../updateunitresults/__init__.php            |  1 -
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/applications/conduit/method/differential/updateunitresults/ConduitAPI_differential_updateunitresults_Method.php b/src/applications/conduit/method/differential/updateunitresults/ConduitAPI_differential_updateunitresults_Method.php
index 5c2b498022..01f0f1fa9c 100644
--- a/src/applications/conduit/method/differential/updateunitresults/ConduitAPI_differential_updateunitresults_Method.php
+++ b/src/applications/conduit/method/differential/updateunitresults/ConduitAPI_differential_updateunitresults_Method.php
@@ -1,7 +1,7 @@
 <?php
 
 /*
- * Copyright 2011 Facebook, Inc.
+ * Copyright 2012 Facebook, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -74,13 +74,11 @@ extends ConduitAPIMethod {
     $postponed_count = 0;
     $unit_status = null;
 
+    // If the test result already exists, then update it with
+    // the new info.
     foreach ($unit_results as &$unit_result) {
-      // Update the results for the test that has the same path.
-      if (($unit_result['name'] === $file ||
-           $unit_result['name'] === $diff->getSourcePath().$file) &&
-          $unit_result['result'] ===
-          DifferentialUnitTestResult::RESULT_POSTPONED) {
-        $unit_result['name'] = $name;
+      if ($unit_result['name'] === $name) {
+        $unit_result['file'] = $file;
         $unit_result['result'] = $result;
         $unit_result['userdata'] = $message;
         $unit_status = $result;
@@ -89,10 +87,15 @@ extends ConduitAPIMethod {
     }
     unset($unit_result);
 
+    // If the test result doesn't exist, just add it.
     if (!$unit_status) {
-      phlog("Could not update test results: {$diff_id} {$file} {$name}".
-            " {$result} {$message}");
-      return;
+      $unit_result = array();
+      $unit_result['file'] = $file;
+      $unit_result['name'] = $name;
+      $unit_result['result'] = $result;
+      $unit_result['userdata'] = $message;
+      $unit_status = $result;
+      $unit_results[] = $unit_result;
     }
 
     $diff_property->setData($unit_results);
diff --git a/src/applications/conduit/method/differential/updateunitresults/__init__.php b/src/applications/conduit/method/differential/updateunitresults/__init__.php
index 405d942cbc..d0e80055ee 100644
--- a/src/applications/conduit/method/differential/updateunitresults/__init__.php
+++ b/src/applications/conduit/method/differential/updateunitresults/__init__.php
@@ -13,7 +13,6 @@ phutil_require_module('phabricator', 'applications/differential/constants/unitte
 phutil_require_module('phabricator', 'applications/differential/storage/diff');
 phutil_require_module('phabricator', 'applications/differential/storage/diffproperty');
 
-phutil_require_module('phutil', 'error');
 phutil_require_module('phutil', 'utils');