Manifest file can contain a test metric which references a non-existent test
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 17 Nov 2018 01:14:26 +0000 (01:14 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 17 Nov 2018 01:14:26 +0000 (01:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191796

Reviewed by Dewei Zhu.

The bug was caused by a race condition between the manifest file fetching the list of tests and test metrics
and new tests and test metrics being added. Because we would fetch tests before test metrics, it was possible
for new test metrics which references a test not included in the fetched tests to be present in the test metrics.

Fixed the bug by changing the order of the queries so that test metrics are fetched before tests. This guarantees
that any test referenced by a test metric always exists and thefore included in the manifest file.

Unfortunately no new tests beucase this involes a race condition.

* public/include/manifest-generator.php:

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/include/manifest-generator.php

index 1e00c18..d06dc0c 100644 (file)
@@ -1,3 +1,21 @@
+2018-11-16  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Manifest file can contain a test metric which references a non-existent test
+        https://bugs.webkit.org/show_bug.cgi?id=191796
+
+        Reviewed by Dewei Zhu.
+
+        The bug was caused by a race condition between the manifest file fetching the list of tests and test metrics
+        and new tests and test metrics being added. Because we would fetch tests before test metrics, it was possible
+        for new test metrics which references a test not included in the fetched tests to be present in the test metrics.
+
+        Fixed the bug by changing the order of the queries so that test metrics are fetched before tests. This guarantees
+        that any test referenced by a test metric always exists and thefore included in the manifest file.
+
+        Unfortunately no new tests beucase this involes a race condition.
+
+        * public/include/manifest-generator.php:
+
 2018-11-13  Dewei Zhu  <dewei_zhu@apple.com>
 
         Add cache for CommitLog objects to avoid refetching same commit.
index bafd825..446fae7 100644 (file)
@@ -26,8 +26,10 @@ class ManifestGenerator {
         foreach ($repositories_with_commit as &$row)
             $row = $row['commit_repository'];
 
-        $tests = (object)$this->tests();
+        // Query test metrics before tests so that every test a test metric references is guaranteed to exist
+        // even if there were new test metrics added by the time we fetched tests.
         $metrics = (object)$this->metrics();
+        $tests = (object)$this->tests();
         $platforms = (object)$this->platforms($platform_table, false);
         $dashboard = (object)$this->platforms($platform_table, true);
         $repositories = (object)$this->repositories($repositories_table, $repositories_with_commit);