Show non-webkit test failures in the failure stream

Tools/GardeningServer/ui/ct-results-detail.html

 <!--
 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.
 -->
 
 <link rel="import" href="ct-results-comparison.html">
 
-<polymer-element name="ct-results-detail" attributes="test builder result">
+<polymer-element name="ct-results-detail" attributes="failure builder">
   <template>
     <style>
       :host {
         display: block;
       }
     </style>
     <template if="{{!_urlGroups.length}}">
       No results to display.
     </template>
     <template repeat="{{urlGroup in _urlGroups}}">
       <template if="{{urlGroup.urls[_kUnknownKind]}}">
         <ct-test-output type="{{urlGroup.type}}" url="{{urlGroup.urls[_kUnknownKind]}}"></ct-test-output>
       </template>
       <template if="{{!urlGroup.urls[_kUnknownKind]}}">
         <ct-results-comparison type="{{urlGroup.type}}" expectedUrl="{{urlGroup.urls[_kExpectedKind]}}"
             actualUrl="{{urlGroup.urls[_kActualKind]}}" diffUrl="{{urlGroup.urls[_kDiffKind]}}"></ct-results-comparison>
       </template>
     </template>
   </template>
   <script>
     Polymer({
-      test: '',
+      failure: null,
       // FIXME: Initializing builder gives a JS error. Presumably because
       // ct-results-by-builder sets builder="{{builders[selected]}}". But,
       // it seems wrong that the way the parent uses this element constrains
       // what the element can do. Polymer bug?
       // builder: '',
-      // FIXME: Initializing result gives a JS error like above.
-      // result: '',
 
       _urlGroups: [],
       _kExpectedKind: results.kExpectedKind,
       _kActualKind: results.kActualKind,
       _kDiffKind: results.kDiffKind,
       _kUnknownKind: results.kUnknownKind,
 
       observe: {
-        result: '_update',
-        test: '_update',
+        failure: '_update',
         builder: '_update',
       },
 
       _update: function() {
-        if (!this.test || !this.builder || !this.result)
+        if (!this.failure || !this.builder)
           return;
 
-        var failureInfo = results.failureInfo(this.test, this.builder, this.result);
+        // FIXME: If the types of groups doesn't change, then it'd be better to do this
+        // update in place so that the user doesn't see a flicker.
+        this._urlGroups = [];
+
+        var result = this.failure.resultNodesByBuilder[this.builder];
+        // FIXME: There's probably a less hacky way to check this.
+        if (result.actual == 'FAIL' || result.actual == 'UNKNOWN')
+          this._updateUrls();
+        else
+          this._updateWebkitTestUrls();
+      },
+
+      _updateWebkitTestUrls: function() {
+        var result = this.failure.resultNodesByBuilder[this.builder];
+        var failureInfo = results.failureInfo(this.failure.testName, this.builder, result.actual);
+
         // FIXME: Move this logic to a proper model class so that the network requests this makes
         // can be easily mocked out in tests.
         results.fetchResultsURLs(failureInfo).then(function(resultsUrls) {
           var resultsUrlsByTypeAndKind = {};
           resultsUrls.forEach(function(url) {
               var resultType = results.resultType(url);
               if (!resultsUrlsByTypeAndKind[resultType])
                   resultsUrlsByTypeAndKind[resultType] = {};
               resultsUrlsByTypeAndKind[resultType][results.resultKind(url)] = url;
           });
 
-          this._urlGroups = [];
           Object.keys(resultsUrlsByTypeAndKind, function(resultType, resultsUrlsByKind) {
             this._urlGroups.push({
               type: resultType,
               urls: resultsUrlsByKind,
             });
           }.bind(this));
         }.bind(this));
       },
+
+      _updateUrls: function() {
+        var result = this.failure.resultNodesByBuilder[this.builder];
+
+        // FIXME: We only store build logs by the test name, not the testsuite.testname,
+        // which means that two failing tests from different test suites conflict!
+        var testPart = result.actual == 'UNKNOWN' ? 'stdio' : this.failure.testName.split('.')[1];
+        var url = result.masterUrl +
+            '/builders/' + encodeURIComponent(this.builder) +
+            '/builds/' + result.lastFailingBuild +
+            '/steps/' + this.failure.step +
+            '/logs/' + testPart;

[abarth-chromium, 2014/07/29 16:38:17]

Should this work be done by some model object?  Having this logic in the view
seems odd.

[ojan, 2014/07/30 04:14:17]

Absolutely, but so should the pre-existing code here (the stuff that's now in
_updateWebkitTestUrls), so I was punting on that since we need to turn this
whole setup into a model object. I'll add a FIXME though to state this
explicitly.

+
+        var resultsUrlsByKind = {};
+        resultsUrlsByKind[this._kUnknownKind] = url;
+
+        this._urlGroups.push({
+          type: results.kTextType,
+          urls: resultsUrlsByKind,
+        });
+      },
     });
   </script>
 </polymer-element>