Take two at fixing duplicate alert failures.

r183289 caused a new overwriting alerts bug because it didn't replicate
the way builder_alerts handled null failure reasons. Using step::reason
won't actually be unique if the reason is null since builder_alerts
doesn't merge failures with null reasons.

R=leviw@chromium.org,jyasskin@chromium.org
NOTRY=true

Review URL: https://codereview.chromium.org/638333002

git-svn-id: svn://svn.chromium.org/blink/trunk@183831 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Tools/GardeningServer/model/ct-step-failure.html

@@ -6,7 +6,18 @@ found in the LICENSE file.
 
 <script>
 function CTStepFailure(step, reason, resultsByBuilder) {
-  this.key = step + '::' + reason;
+  this.key = step + '::'
+
+  // The key needs to be unique, so disambiguate null reason
+  // failures by their builders, but don't do regular failures
+  // by builders because we want failures with reasons to update
+  // the existing failure group instead of causing the group to
+  // get recreated since throwing it away loses in app state.
+  if (reason)
+    this.key += reason;
+  else
+    this.key += Object.keys(resultsByBuilder).join('::');
+
   this.step = step;
   // FIXME: Rename this to reason.
   this.testName = reason;

third_party/WebKit/Tools/GardeningServer/model/test/ct-failures-tests.html

@@ -270,6 +270,26 @@ describe('ct-failures', function() {
                 chromium: 199,
               },
             },
+            {
+              sort_key: 'mac: Mac Tests',
+              failure_keys: [ 'f3', ],
+              likely_revisions: [
+                'chromium: 100',
+                'chromium: 101',
+              ],
+              merged_first_failing: {
+                blink: 50,
+                nacl: 50,
+                v8: 50,
+                chromium: 200,
+              },
+              merged_last_passing: {
+                blink: 50,
+                nacl: 50,
+                v8: 50,
+                chromium: 199,
+              },
+            },
           ],
           alerts: [
             {
@@ -334,6 +354,37 @@ describe('ct-failures', function() {
               },
               would_close_tree: true
             },
+            {
+              last_result_time: 1409697347.089103,
+              passing_build: 2485,
+              last_failing_build: 2489,
+              failing_build: 2486,
+              latest_revisions: {
+                v8: 50,
+                chromium: 103,
+                nacl: 50,
+                blink: 51
+              },
+              master_url: "https://build.chromium.org/p/chromium.mac",
+              reason: null,
+              failing_revisions: {
+                v8: 50,
+                chromium: 101,
+                nacl: 50,
+                blink: 50
+              },
+              builder_name: "Mac Tests",
+              key: "f3",
+              step_name: "compile",
+              tree: 'chromium',
+              passing_revisions: {
+                v8: 50,
+                chromium: 99,
+                nacl: 50,
+                blink: 50
+              },
+              would_close_tree: true
+            },
           ],
           date: 1409697816.726562,
         },
@@ -355,16 +406,17 @@ describe('ct-failures', function() {
         var analyzer = new CTFailures(CTCommitLogMock());
         return analyzer.update().then(function() {
           assert.property(analyzer.failures, 'chromium');
-          assert.lengthOf(analyzer.failures.chromium, 3);
+          assert.lengthOf(analyzer.failures.chromium, 4);
           var masterFailureGroup = analyzer.failures.chromium[0];
           var testFailureGroup = analyzer.failures.chromium[1];
           var compileFailureGroup = analyzer.failures.chromium[2];
+          var macCompileFailureGroup = analyzer.failures.chromium[3];
           assert.equal(compileFailureGroup.constructor, CTFailureGroup);
-          assert.equal(compileFailureGroup.key, 'compile::null');
+          assert.equal(compileFailureGroup.key, 'compile::Linux Tests (dbg)(1)');
           assert.lengthOf(compileFailureGroup.data.failures, 1);
           var failure = compileFailureGroup.data.failures[0];
           assert.equal(failure.constructor, CTStepFailure);
-          assert.equal(failure.key, 'compile::null');
+          assert.equal(failure.key, 'compile::Linux Tests (dbg)(1)');
           assert.equal(failure.step, 'compile');
           assert.equal(failure.testName, null);
           var resultNodesByBuilder = failure.resultNodesByBuilder;
@@ -395,6 +447,7 @@ describe('ct-failures', function() {
             assert.strictEqual(analyzer.failures.chromium[0], masterFailureGroup)
             assert.strictEqual(analyzer.failures.chromium[1], testFailureGroup)
             assert.strictEqual(analyzer.failures.chromium[2], compileFailureGroup)
+            assert.strictEqual(analyzer.failures.chromium[3], macCompileFailureGroup)
             assert.strictEqual(compileFailureGroup.data.failures[0], failure)
             assert.strictEqual(failure.resultNodesByBuilder, resultNodesByBuilder);
             assert.strictEqual(resultNodesByBuilder['Linux Tests (dbg)(1)'], dbgBuilder);

third_party/WebKit/Tools/GardeningServer/model/test/ct-step-failure-tests.html

@@ -11,7 +11,7 @@ found in the LICENSE file.
 
 var assert = chai.assert;
 
-describe('ct-failure model', function() {
+describe('CTStepFailure', function() {
   var tests = [
     {
       failure: new CTStepFailure('browser_tests', 'FooTest.Bar', {}, 123, 123),
@@ -24,9 +24,9 @@ describe('ct-failure model', function() {
       expectedKey: 'webkit_tests::fast/text/foo.html',
     },
     {
-      failure: new CTStepFailure('compile', undefined, {}, 123, 123),
+      failure: new CTStepFailure('compile', undefined, {builder1: {}}, 123, 123),
       expectedGroupName: undefined,
-      expectedKey: 'compile::undefined',
+      expectedKey: 'compile::builder1',
     }
   ];