Showing lists of flaky tests for a builder takes too long
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 25 Oct 2013 04:22:17 +0000 (04:22 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 25 Oct 2013 04:22:17 +0000 (04:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=123311

Reviewed by Sam Weinig.

Generate JSONs for tests failing, flaky, or with wrong expectation at the time a builder reports results
instead when the frontend requests to those those results since it takes multiple seconds or minutes to
generate those JSON files.

* api/failing-tests.php: Moved and renamed to manually generate all JSON files for a given builder.
(main):

* api/report.php: Manually flush and end the request (to avoid blocking run-webkit-tests on the other side
for minutes), then generate JSONs for tests that are failing, are flaky, and have wrong expectations.

* api/results.php: Merge format_result_rows here since it's not used anywhere else.

* include/config.json: Added the path to the data directory into which JSON files are generated.

* include/db.php:
(configPath): Takes a relative path value from config.json, and resolves it.

* include/json-shared.php:
(echo_success): Extracted from exit_with_success.
(exit_with_success):

* include/test-results.php:
(add_builder): Extracted from add_build.
(add_build):
(ResultsJSONWriter): Extracted from api/failing-tests.php.
(ResultsJSONWriter.__construct):
(ResultsJSONWriter.start):
(ResultsJSONWriter.end):
(ResultsJSONWriter.add_results_for_test_if_matches):
(ResultsJSONWriter.pass_for_failure_type):
(FailingResultsJSONWriter): Extracted from index.html's TestResultsView._matchesFailureType.
(FailingResultsJSONWriter.__construct):
(FailingResultsJSONWriter.pass_for_failure_type):
(FlakyResultsJSONWriter): Ditto.
(FlakyResultsJSONWriter.__construct):
(FlakyResultsJSONWriter.pass_for_failure_type):
(WrongExpectationsResultsJSONWriter): Ditto.
(WrongExpectationsResultsJSONWriter.__construct):
(WrongExpectationsResultsJSONWriter.pass_for_failure_type):
(ResultsJSONGenerator): Ditto.
(ResultsJSONGenerator.__construct):
(ResultsJSONGenerator.generate):
(ResultsJSONGenerator.open_json_for_failure_type):
(ResultsJSONGenerator.write_jsons):

* index.html:
(TestResultsView):
(TestResultsView.setBuilders):
(TestResultsView._createResultCell): Add a hyperlink to results.html in the tooltip.
(TestResultsView.fetchFailingTestsForBuilder): Fetch the generated JSON files.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@157988 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Websites/test-results/ChangeLog
Websites/test-results/api/failing-tests.php
Websites/test-results/api/report.php
Websites/test-results/api/results.php
Websites/test-results/include/config.json
Websites/test-results/include/db.php
Websites/test-results/include/json-shared.php
Websites/test-results/include/test-results.php
Websites/test-results/index.html

index 991cd9f..287a72b 100644 (file)
@@ -1,3 +1,61 @@
+2013-10-24  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Showing lists of flaky tests for a builder takes too long
+        https://bugs.webkit.org/show_bug.cgi?id=123311
+
+        Reviewed by Sam Weinig.
+
+        Generate JSONs for tests failing, flaky, or with wrong expectation at the time a builder reports results
+        instead when the frontend requests to those those results since it takes multiple seconds or minutes to
+        generate those JSON files.
+
+        * api/failing-tests.php: Moved and renamed to manually generate all JSON files for a given builder.
+        (main):
+
+        * api/report.php: Manually flush and end the request (to avoid blocking run-webkit-tests on the other side
+        for minutes), then generate JSONs for tests that are failing, are flaky, and have wrong expectations.
+
+        * api/results.php: Merge format_result_rows here since it's not used anywhere else.
+
+        * include/config.json: Added the path to the data directory into which JSON files are generated.
+
+        * include/db.php:
+        (configPath): Takes a relative path value from config.json, and resolves it.
+
+        * include/json-shared.php:
+        (echo_success): Extracted from exit_with_success.
+        (exit_with_success):
+
+        * include/test-results.php:
+        (add_builder): Extracted from add_build.
+        (add_build):
+        (ResultsJSONWriter): Extracted from api/failing-tests.php.
+        (ResultsJSONWriter.__construct):
+        (ResultsJSONWriter.start):
+        (ResultsJSONWriter.end):
+        (ResultsJSONWriter.add_results_for_test_if_matches):
+        (ResultsJSONWriter.pass_for_failure_type):
+        (FailingResultsJSONWriter): Extracted from index.html's TestResultsView._matchesFailureType.
+        (FailingResultsJSONWriter.__construct):
+        (FailingResultsJSONWriter.pass_for_failure_type):
+        (FlakyResultsJSONWriter): Ditto.
+        (FlakyResultsJSONWriter.__construct):
+        (FlakyResultsJSONWriter.pass_for_failure_type):
+        (WrongExpectationsResultsJSONWriter): Ditto.
+        (WrongExpectationsResultsJSONWriter.__construct):
+        (WrongExpectationsResultsJSONWriter.pass_for_failure_type):
+        (ResultsJSONGenerator): Ditto.
+        (ResultsJSONGenerator.__construct):
+        (ResultsJSONGenerator.generate):
+        (ResultsJSONGenerator.open_json_for_failure_type):
+        (ResultsJSONGenerator.write_jsons):
+
+        * index.html:
+        (TestResultsView):
+        (TestResultsView.setBuilders):
+        (TestResultsView._createResultCell): Add a hyperlink to results.html in the tooltip.
+        (TestResultsView.fetchFailingTestsForBuilder): Fetch the generated JSON files.
+
 2013-10-23  Ryosuke Niwa  <rniwa@webkit.org>
 
         Reverted erroneously committed changes from the previous commit.
index c57129f..0ee6691 100644 (file)
@@ -3,48 +3,24 @@
 require_once('../include/json-shared.php');
 require_once('../include/test-results.php');
 
-$db = connect();
-
-require_existence_of($_GET, array('builder' => '/^[A-Za-z0-9 \(\)\-_]+$/'));
-$builder_name = $_GET['builder'];
-$number_of_days = array_get($_GET, 'days');
-if ($number_of_days) {
-    require_format('number_of_days', $number_of_days, '/^[0-9]+$/');
-    $number_of_days = intval($number_of_days);
-} else
-    $number_of_days = 3;
-
-$builder_row = $db->select_first_row('builders', NULL, array('name' => $builder_name));
-if (!$builder_row)
-    exit_with_error('BuilderNotFound');
-$builder_id = $builder_row['id'];
-
-$all_results = $db->query(
-"SELECT results.*, builds.*, array_agg((build_revisions.repository, build_revisions.value, build_revisions.time)) AS revisions
-    FROM results, builds, build_revisions
-    WHERE build_revisions.build = builds.id AND results.build = builds.id AND builds.builder = $1
-    AND builds.start_time > now() - interval '$number_of_days days'
-    GROUP BY results.id, builds.id ORDER BY results.test, max(build_revisions.time) DESC", array($builder_id));
-
-if (!$all_results)
-    exit_with_error('ResultsNotFound');
-
-// To conserve memory, we serialize tests at a time.
-echo "{\"status\": \"OK\", \"builders\": {\"$builder_id\":{";
-$currentTest = NULL;
-$i = 0;
-while ($result = $db->fetch_next_row($all_results)) {
-    if ($result['test'] != $currentTest) {
-        if ($currentTest)
-            echo '],';
-        $currentTest = $result['test'];
-        echo "\"$currentTest\": [";
-    } else
-        echo ',';
-    echo json_encode(format_result($result), true);
+function main() {
+    require_existence_of($_GET, array('builder' => '/^[A-Za-z0-9 \(\)\-_]+$/'));
+    $builder_name = $_GET['builder'];
+
+    $db = connect();
+    $builder_row = $db->select_first_row('builders', NULL, array('name' => $builder_name));
+    if (!$builder_row)
+        exit_with_error('BuilderNotFound');
+    $builder_id = $builder_row['id'];
+
+    $generator = new ResultsJSONGenerator($db, $builder_id);
+
+    if ($generator->generate())
+        exit_with_success();
+    else
+        exit_with_error('ResultsNotFound');
 }
-if ($currentTest)
-    echo ']';
-echo '}}}';
+
+main();
 
 ?>
index db6ba6c..dfa3458 100644 (file)
@@ -13,6 +13,9 @@ require_existence_of($_POST, array(
     'revisions' => '/^.+?$/',
     'start_time' => '/^[0-9]+(\.[0-9]+)?$/',
     'end_time' => '/^[0-9]+(\.[0-9]+)?$/'));
+$master = $_POST['master'];
+$builder_name = $_POST['builder_name'];
+$build_number = intval($_POST['build_number']);
 
 if (!array_key_exists('file', $_FILES) or !array_key_exists('tmp_name', $_FILES['file']) or count($_FILES['file']['tmp_name']) <= 0)
     exit_with_error('ResultsJSONNotIncluded');
@@ -33,9 +36,13 @@ if (!$test_results)
 $start_time = float_to_time($_POST['start_time']);
 $end_time = float_to_time($_POST['end_time']);
 
-$build_id = add_build($db, $_POST['master'], $_POST['builder_name'], intval($_POST['build_number']));
+$builder_id = add_builder($db, $master, $builder_name);
+if (!$builder_id)
+    exit_with_error('FailedToInsertBuilder', array('master' => $master, 'builderName' => $builder_name));
+
+$build_id = add_build($db, $builder_id, $build_number);
 if (!$build_id)
-    exit_with_error('FailedToInsertBuild', array('master' => $_POST['master'], 'builderName' => $_POST['builder_name'], 'buildNumber' => $_POST['build_number']));
+    exit_with_error('FailedToInsertBuild', array('builderId' => $builder_id, 'buildNumber' => $build_number));
 
 foreach ($revisions as $repository_name => $revision_data) {
     $repository_id = $db->select_or_insert_row('repositories', NULL, array('name' => $repository_name));
@@ -55,6 +62,14 @@ $slave_id = add_slave($db, $_POST['build_slave']);
 if (!store_test_results($db, $test_results, $build_id, $start_time, $end_time, $slave_id))
     exit_with_error('FailedToStoreResults', array('buildId' => $build_id));
 
-exit_with_success();
+echo_success();
+
+ob_end_flush();
+flush();
+if (function_exists('fastcgi_finish_request'))
+    fastcgi_finish_request();
+
+$generator = new ResultsJSONGenerator($db, $builder_id);
+$generator->generate();
 
 ?>
index 1883ea0..291d809 100644 (file)
@@ -19,6 +19,10 @@ $result_rows = $db->query_and_fetch_all(
 if (!$result_rows)
     exit_with_error('ResultsNotFound');
 
-exit_with_success(format_result_rows($result_rows));
+$builders = array();
+foreach ($result_rows as $result)
+    array_push(array_ensure_item_has_array($builders, $result['builder']), format_result($result));
+
+exit_with_success(array('builders' => $builders));
 
 ?>
index 3f45f02..6e3687c 100644 (file)
@@ -1,6 +1,7 @@
 {
     "debug": true,
     "jsonCacheMaxAge": 600,
+    "dataDirectory": "../data",
     "database": {
         "host": "localhost",
         "port": "5432",
index e9968f8..786d977 100644 (file)
@@ -33,6 +33,15 @@ function config($key) {
     return $_config[$key];
 }
 
+function configPath($key, $additional_path = '') {
+    $relative_path = config($key);
+    if (!$relative_path)
+        return NULL;
+    if ($additional_path)
+        $additional_path = '/' . $additional_path;
+    return realpath(dirname(__FILE__) . "/$relative_path") . $additional_path;
+}
+
 if (config('debug'))
     ini_set('display_errors', 'On');
 
index a999217..e91903c 100644 (file)
@@ -13,9 +13,13 @@ function exit_with_error($status, $details = array()) {
     exit(1);
 }
 
-function exit_with_success($details = array()) {
+function echo_success($details = array()) {
     $details['status'] = 'OK';
     echo json_encode($details);
+}
+
+function exit_with_success($details = array()) {
+    echo_success($details);
     exit(0);
 }
 
index 423d21c..7367844 100644 (file)
@@ -8,14 +8,14 @@ function float_to_time($time_in_float) {
     return $time;
 }
 
-function add_build($db, $master, $builder_name, $build_number) {
+function add_builder($db, $master, $builder_name) {
     if (!in_array($master, config('masters')))
         return NULL;
 
-    $builder_id = $db->select_or_insert_row('builders', NULL, array('master' => $master, 'name' => $builder_name));
-    if (!$builder_id)
-        return NULL;
+    return $db->select_or_insert_row('builders', NULL, array('master' => $master, 'name' => $builder_name));
+}
 
+function add_build($db, $builder_id, $build_number) {
     return $db->select_or_insert_row('builds', NULL, array('builder' => $builder_id, 'number' => $build_number));
 }
 
@@ -105,13 +105,165 @@ function format_result($result) {
         'modifiers' => $result['modifiers']);
 }
 
-function format_result_rows($result_rows) {
-    $builders = array();
-    foreach ($result_rows as $result) {
-        array_push(array_ensure_item_has_array(array_ensure_item_has_array($builders, $result['builder']), $result['test']),
-            format_result($result));
+abstract class ResultsJSONWriter {
+    private $fp;
+    private $emitted_results;
+
+    public function __construct($fp) {
+        $this->fp = $fp;
+        $this->emitted_results = FALSE;
+    }
+
+    public function start($builder_id) {
+        fwrite($this->fp, "{\"status\": \"OK\", \"builders\": {\"$builder_id\":{");
+    }
+
+    public function end($total_time) {
+        fwrite($this->fp, "}}, \"totalGenerationTime\": $total_time}");
+    }
+
+    public function add_results_for_test_if_matches($current_test, $current_results) {
+        if (!count($current_results) || $this->pass_for_failure_type($current_results))
+            return;
+        // FIXME: Why do we need to check the count?
+
+        $prefix = $this->emitted_results ? ",\n" : "";
+        fwrite($this->fp, "$prefix\"$current_test\":");
+        fwrite($this->fp, json_encode($current_results, true));
+
+        $this->emitted_results = TRUE;
+    }
+
+    abstract protected function pass_for_failure_type(&$results);
+}
+
+class FailingResultsJSONWriter extends ResultsJSONWriter {
+    public function __construct($fp) { parent::__construct($fp); }
+    protected function pass_for_failure_type(&$results) {
+        return $results[0]['actual'] == 'PASS';
+    }
+}
+
+class FlakyResultsJSONWriter extends ResultsJSONWriter {
+    public function __construct($fp) { parent::__construct($fp); }
+    protected function pass_for_failure_type(&$results) {
+        $last_index = count($results) - 1;
+        for ($i = 1; $i < $last_index; $i++) {
+            $previous_actual = $results[$i - 1]['actual'];
+            $next_actual = $results[$i + 1]['actual'];
+            if ($previous_actual == $next_actual && $results[$i]['actual'] != $previous_actual) {
+                $results[$i]['oneOffChange'] = TRUE;
+                return FALSE;
+            }
+        }
+        return TRUE;
+    }
+}
+
+class WrongExpectationsResultsJSONWriter extends ResultsJSONWriter {
+    public function __construct($fp) { parent::__construct($fp); }
+    protected function pass_for_failure_type(&$results) {
+        $latest_expected_result = $results[0]['expected'];
+        $latest_actual_result = $results[0]['actual'];
+
+        if ($latest_expected_result == $latest_actual_result)
+            return TRUE;
+
+        $tokens = explode(' ', $latest_expected_result);
+        return array_search($latest_actual_result, $tokens) !== FALSE
+            || (($latest_actual_result == 'TEXT' || $latest_actual_result == 'TEXT+IMAGE') && array_search('FAIL', $tokens) !== FALSE);
+    }
+}
+
+class ResultsJSONGenerator {
+    private $db;
+    private $builder_id;
+
+    const MAXIMUM_NUMBER_OF_DAYS = 30;
+
+    public function __construct($db, $builder_id)
+    {
+        $this->db = $db;
+        $this->builder_id = $builder_id;
+    }
+
+    public function generate()
+    {
+        $start_time = microtime(true);
+
+        if (!$this->builder_id)
+            return FALSE;
+
+        $number_of_days = self::MAXIMUM_NUMBER_OF_DAYS;
+        $all_results = $this->db->query(
+        "SELECT results.*, builds.* FROM results
+            JOIN (SELECT builds.*, array_agg((build_revisions.repository, build_revisions.value, build_revisions.time)) AS revisions
+                    FROM builds, build_revisions
+                    WHERE build_revisions.build = builds.id AND builds.builder = $1 AND builds.start_time > now() - interval '$number_of_days days'
+                    GROUP BY builds.id
+                    ORDER BY max(build_revisions.time) DESC) as builds ON results.build = builds.id
+            ORDER BY results.test", array($this->builder_id));
+        if (!$all_results)
+            return FALSE;
+
+        $failing_json_fp = $this->open_json_for_failure_type('failing');
+        try {
+            $flaky_json_fp = $this->open_json_for_failure_type('flaky');
+            try {
+                $wrongexpectations_json_fp = $this->open_json_for_failure_type('wrongexpectations');
+                try {
+                    return $this->write_jsons($all_results, array(
+                        new FailingResultsJSONWriter($failing_json_fp),
+                        new FlakyResultsJSONWriter($flaky_json_fp),
+                        new WrongExpectationsResultsJSONWriter($wrongexpectations_json_fp)), $start_time);
+                } catch (Exception $exception) {
+                    fclose($wrongexpectations_json_fp);
+                    throw $exception;
+                }
+            } catch (Exception $exception) {
+                fclose($flaky_json_fp);
+                throw $exception;
+            }
+        } catch (Exception $exception) {
+            fclose($failing_json_fp);
+            throw $exception;
+        }
+        return FALSE;
+    }
+
+    private function open_json_for_failure_type($failure_type) {
+        $failing_json_path = configPath('dataDirectory', $this->builder_id . "-$failure_type.json");
+        if (!$failing_json_path)
+            exit_with_error('FailedToDetermineResultsJSONPath', array('builderId' => $this->builder_id, 'failureType' => $failure_type));
+        $fp = fopen($failing_json_path, 'w');
+        if (!$fp)
+            exit_with_error('FailedToOpenResultsJSON', array('builderId' => $this->builder_id, 'failureType' => $failure_type));
+        return $fp;
+    }
+
+    private function write_jsons($all_results, $writers, $start_time) {
+        foreach ($writers as $writer)
+            $writer->start($this->builder_id);
+        $current_test = NULL;
+        $current_results = array();
+        while ($result = $this->db->fetch_next_row($all_results)) {
+            if ($result['test'] != $current_test) {
+                if ($current_test) {
+                    foreach ($writers as $writer)
+                        $writer->add_results_for_test_if_matches($current_test, $current_results);
+                }
+                $current_results = array();
+                $current_test = $result['test'];
+            }
+            array_push($current_results, format_result($result));
+        }
+
+        $total_time = microtime(true) - $start_time;
+        foreach ($writers as $writer)
+            $writer->end($total_time);
+
+        return TRUE;
     }
-    return array('builders' => $builders);
 }
 
 ?>
index 66fd109..6972ba5 100644 (file)
 <div id="buildersView">
 <form>
 Show
-<label>tests <select id="builderFailureTypeView"><option>failing</option><option>flaky</option><option value="wrongexpectations">has wrong expectations</option></select></label>
+<label>all tests <select id="builderFailureTypeView"><option>failing</option><option>flaky</option><option value="wrongexpectations">has wrong expectations</option></select></label>
 <label>on <select id="builderListView"><option value="">Select builder</option></select></label>
-<label for="builderDaysView">in the last <select id="builderDaysView"><option>5</option><option>15</option><option>30</option></select> days</label>
+<label for="builderDaysView">for
+<select id="builderDaysView" disabled><option>5</option><option>15</option><option selected>30</option></select> days</label>
 </form>
 <div id="builderFailingTestsView"></div>
 </div>
@@ -60,6 +61,7 @@ var TestResultsView = new (function () {
     this._currentBuilderDays = null;
     this._oldHash = null;
     this._builders = {};
+    this._builderByName = {};
     this._slaves = {};
     this._repositories = {};
     this._testCategories = {};
@@ -71,6 +73,10 @@ TestResultsView.setAvailableTests = function (availableTests) {
 
 TestResultsView.setBuilders = function (builders) {
     this._builders = builders;
+    for (var builderId in builders) {
+        var builder = builders[builderId];
+        this._builderByName[builder.name] = builder;
+    }
 }
 
 TestResultsView.setSlaves = function (slaves) {
@@ -141,7 +147,8 @@ TestResultsView._createResultCell = function (master, builder, result, previousR
             element('ul', [
                 element('li', ['Build Time: ' + result.build.formattedBuildTime()]),
                 element('li', ['Revision: '].concat(revisionDescription)),
-                element('li', ['Build: ', element('a', {'href': result.build.buildUrl()}, [buildNumber])]),
+                element('li', ['Build: ', element('a', {'href': result.build.buildUrl()}, [buildNumber]), ' (',
+                    element('a', {'href': resultsPage}, ['results']), ')']),
                 element('li', ['Actual: ' + actual]),
                 element('li', ['Expected: ' + expected]),
             ])
@@ -337,37 +344,6 @@ TestResultsView.fetchTests = function (testNames, doNotUpdateHash) {
         this.updateLocationHash();
 }
 
-TestResultsView._matchesFailureType = function (results, failureType, tn) {
-    if (!results.length)
-        return false;
-    var latestActualResult = results[0].actual;
-    var latestExpectedResult = results[0].expected;
-    switch (failureType) {
-    case 'failing':
-        return results[0].actual != 'PASS';
-    case 'flaky':
-        var offOneChangeCount = 0;
-        for (var i = 1; i + 1 < results.length; i++) {
-            var previousActual = results[i - 1].actual;
-            var nextActual = results[i + 1].actual;
-            if (previousActual == nextActual && results[i].actual != previousActual)
-                offOneChangeCount++;
-        }
-        return offOneChangeCount; // Heuristics.
-    case 'wrongexpectations':
-        if (latestExpectedResult == latestActualResult)
-            return false;
-        var expectedTokens = latestExpectedResult.split(' ');
-        if (expectedTokens.indexOf(latestActualResult) >= 0)
-            return false;
-        if (latestActualResult == 'TEXT' || latestActualResult == 'TEXT+IMAGE' && expectedTokens.indexOf('FAIL') >= 0)
-            return false;
-        return true;
-    }
-
-    return false;
-}
-
 TestResultsView._populateBuilderPane = function(builderName, failureType, results, section) {
     var table = element('table', {'class': 'resultsTable tablesorter'}, [element('caption', [builderName])]);
     var resultsByBuilder = results['builders'];
@@ -387,7 +363,7 @@ TestResultsView._populateBuilderPane = function(builderName, failureType, result
     var self = this;
     for (var testId in resultsByTests) {
         var results = resultsByTests[testId];
-        if (!results.length || !this._matchesFailureType(results, failureType, this._availableTests[testId].name))
+        if (!results.length)
             continue;
         this._createBuildsAndComputeSlownessOfResults(builderId, resultsByTests[testId]);
         var test = this._availableTests[testId];
@@ -416,8 +392,13 @@ TestResultsView.fetchFailingTestsForBuilder = function (builderName, numberOfDay
 
     var self = this;
     var xhr = new XMLHttpRequest();
-    xhr.open("GET", 'api/failing-tests.php?builder=' + escape(builderName) + '&days=' + numberOfDays, true);  
+    var builderId = this._builderByName[builderName].id;
+    xhr.open('GET', 'data/' + builderId + '-' + failureType + '.json', true);  
     xhr.onload = function(event) {
+        if (xhr.status != 200) {
+            section.appendChild(text('Failed to load results for ' + builderName + ': ' + xhr.status));
+            return;
+        }
         var response = JSON.parse(xhr.response);
         section.innerHTML = '';
         if (response['status'] != 'OK') {