Extend commits table to contain testability information.
authordewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Dec 2018 02:05:39 +0000 (02:05 +0000)
committerdewei_zhu@apple.com <dewei_zhu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Dec 2018 02:05:39 +0000 (02:05 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191557

Reviewed by Ryosuke Niwa.

Added the ability to store testability message in commits table.
Refactored '/api/report-commits' to support update commit only.
Updated os version syncing script to be able to update testability information.

* init-database.sql: Added 'commit_testability' field to 'commits' table.
* public/api/report-commits.php: Refactor this api to allow only update existing commits.
* public/include/commit-log-fetcher.php: Expose testability warning information in this API.
* public/include/commit-updater.php: Added 'CommitUpdater' to manage commit insert and update.
* public/v3/models/commit-log.js: CommitLog object should expose testability warning information.
(CommitLog.prototype.updateSingleton):
(CommitLog.prototype.testability):
* public/v3/models/commit-set.js: Added 'commitsWithTestability' and 'commits' to CommitSet and
IntermediateCommitSet instances.
(CommitSet.prototype.commitsWithTestability):
(CommitSet.prototype.commits):
(IntermediateCommitSet.prototype.commitsWithTestability):
(IntermediateCommitSet.prototype.commits):
* server-tests/api-report-commits-tests.js: Added unit tests for '/api/report-commits' when `insert=false`.
* server-tests/tools-os-build-fetcher-tests.js: Added and updated unit tests.
* tools/js/os-build-fetcher.js: Added the ability to update commit testability warnings.
(async.fetchReportAndUpdateCommits):
(prototype.async.fetchReportAndUpdateBuilds):
(prototype.async._fetchAvailableBuilds):
(prototype.async._commitsForAvailableBuilds):
(prototype._commitsWithinRange):
(prototype.async._reportCommits):
(fetchAndReportAllInOrder): Deleted.
(prototype.fetchAndReportNewBuilds): Deleted.
(prototype._fetchAvailableBuilds): Deleted.
(prototype._commitsForAvailableBuilds): Deleted.
(prototype._submitCommits): Deleted.
* tools/sync-os-versions.js:
* unit-tests/commit-log-tests.js: Added a unit test for 'testability'.
* unit-tests/commit-set-tests.js: Added unit tests for 'commitsWithTestability'.

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

13 files changed:
Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/init-database.sql
Websites/perf.webkit.org/public/api/report-commits.php
Websites/perf.webkit.org/public/include/commit-log-fetcher.php
Websites/perf.webkit.org/public/include/commit-updater.php [new file with mode: 0644]
Websites/perf.webkit.org/public/v3/models/commit-log.js
Websites/perf.webkit.org/public/v3/models/commit-set.js
Websites/perf.webkit.org/server-tests/api-report-commits-tests.js
Websites/perf.webkit.org/server-tests/tools-os-build-fetcher-tests.js
Websites/perf.webkit.org/tools/js/os-build-fetcher.js
Websites/perf.webkit.org/tools/sync-os-versions.js
Websites/perf.webkit.org/unit-tests/commit-log-tests.js
Websites/perf.webkit.org/unit-tests/commit-set-tests.js

index d06dc0c..0936909 100644 (file)
@@ -1,3 +1,45 @@
+2018-12-14  Dewei Zhu  <dewei_zhu@apple.com>
+
+        Extend commits table to contain testability information.
+        https://bugs.webkit.org/show_bug.cgi?id=191557
+
+        Reviewed by Ryosuke Niwa.
+
+        Added the ability to store testability message in commits table.
+        Refactored '/api/report-commits' to support updating commit only.
+        Updated os version syncing script to be able to update testability information.
+
+        * init-database.sql: Added 'commit_testability' field to 'commits' table.
+        * public/api/report-commits.php: Refactor this api to allow only update existing commits.
+        * public/include/commit-log-fetcher.php: Expose testability warning information in this API.
+        * public/include/commit-updater.php: Added 'CommitUpdater' to manage commit insert and update.
+        * public/v3/models/commit-log.js: CommitLog object should expose testability warning information.
+        (CommitLog.prototype.updateSingleton):
+        (CommitLog.prototype.testability):
+        * public/v3/models/commit-set.js: Added 'commitsWithTestability' and 'commits' to CommitSet and
+        IntermediateCommitSet instances.
+        (CommitSet.prototype.commitsWithTestability):
+        (CommitSet.prototype.commits):
+        (IntermediateCommitSet.prototype.commitsWithTestability):
+        (IntermediateCommitSet.prototype.commits):
+        * server-tests/api-report-commits-tests.js: Added unit tests for '/api/report-commits' when `insert=false`.
+        * server-tests/tools-os-build-fetcher-tests.js: Added and updated unit tests.
+        * tools/js/os-build-fetcher.js: Added the ability to update commit testability warnings.
+        (async.fetchReportAndUpdateCommits):
+        (prototype.async.fetchReportAndUpdateBuilds):
+        (prototype.async._fetchAvailableBuilds):
+        (prototype.async._commitsForAvailableBuilds):
+        (prototype._commitsWithinRange):
+        (prototype.async._reportCommits):
+        (fetchAndReportAllInOrder): Deleted.
+        (prototype.fetchAndReportNewBuilds): Deleted.
+        (prototype._fetchAvailableBuilds): Deleted.
+        (prototype._commitsForAvailableBuilds): Deleted.
+        (prototype._submitCommits): Deleted.
+        * tools/sync-os-versions.js:
+        * unit-tests/commit-log-tests.js: Added a unit test for 'testability'.
+        * unit-tests/commit-set-tests.js: Added unit tests for 'commitsWithTestability'.
+
 2018-11-16  Ryosuke Niwa  <rniwa@webkit.org>
 
         Manifest file can contain a test metric which references a non-existent test
index 8161efe..c7b7d60 100644 (file)
@@ -101,6 +101,7 @@ CREATE TABLE commits (
     commit_committer integer REFERENCES committers ON DELETE CASCADE,
     commit_message text,
     commit_reported boolean NOT NULL DEFAULT FALSE,
+    commit_testability varchar(64) DEFAULT NULL,
     CONSTRAINT commit_in_repository_must_be_unique UNIQUE(commit_repository, commit_revision));
 CREATE INDEX commit_time_index ON commits(commit_time);
 CREATE INDEX commit_order_index ON commits(commit_order);
index 6c651a9..da411ad 100644 (file)
@@ -1,6 +1,7 @@
 <?php
 
 require('../include/json-header.php');
+require('../include/commit-updater.php');
 
 function main($post_data)
 {
@@ -12,93 +13,15 @@ function main($post_data)
 
     verify_slave($db, $report);
 
-    $commits = array_get($report, 'commits', array());
+    $commit_info_list = array_get($report, 'commits', array());
+    $should_insert = array_get($report, 'insert', true);
 
-    foreach ($commits as $commit_info) {
-        if (!array_key_exists('repository', $commit_info))
-            exit_with_error('MissingRepositoryName', array('commit' => $commit_info));
-        if (!array_key_exists('revision', $commit_info))
-            exit_with_error('MissingRevision', array('commit' => $commit_info));
-        require_format('Revision', $commit_info['revision'], '/^[A-Za-z0-9 \.]+$/');
-        if (array_key_exists('author', $commit_info) && !is_array($commit_info['author']))
-            exit_with_error('InvalidAuthorFormat', array('commit' => $commit_info));
-    }
-
-    $db->begin_transaction();
-    foreach ($commits as $commit_info) {
-        $repository_id = $db->select_or_insert_row('repositories', 'repository', array('name' => $commit_info['repository'], 'owner' => NULL));
-        if (!$repository_id) {
-            $db->rollback_transaction();
-            exit_with_error('FailedToInsertRepository', array('commit' => $commit_info));
-        }
-        $owner_commit_id = insert_commit($db, $commit_info, $repository_id, NULL);
-        if (!array_key_exists('ownedCommits', $commit_info))
-            continue;
-
-        foreach($commit_info['ownedCommits'] as $owned_commit_repository_name => $owned_commit_info) {
-            if (array_key_exists('time', $owned_commit_info)) {
-                $db->rollback_transaction();
-                exit_with_error('OwnedCommitShouldNotContainTimestamp', array('commit' => $owned_commit_info));
-            }
-            $owned_commit_repository_id = $db->select_or_insert_row('repositories', 'repository', array('name' => $owned_commit_repository_name, 'owner' => $repository_id));
-            if (!$owned_commit_repository_id) {
-                $db->rollback_transaction();
-                exit_with_error('FailedToInsertRepository', array('commit' => $owned_commit_info));
-            }
-            insert_commit($db, $owned_commit_info, $owned_commit_repository_id, $owner_commit_id);
-        }
-    }
-    $db->commit_transaction();
+    $commit_modifier = new CommitUpdater($db);
+    $commit_modifier->report_commits($commit_info_list, $should_insert);
 
     exit_with_success();
 }
 
-function insert_commit($db, $commit_info, $repository_id, $owner_commit_id)
-{
-    $author = array_get($commit_info, 'author');
-    $committer_id = NULL;
-    if ($author) {
-        $account = array_get($author, 'account');
-        $committer_query = array('repository' => $repository_id, 'account' => $account);
-        $committer_data = $committer_query;
-        $name = array_get($author, 'name');
-        if ($name)
-            $committer_data['name'] = $name;
-        $committer_id = $db->update_or_insert_row('committers', 'committer', $committer_query, $committer_data);
-        if (!$committer_id) {
-            $db->rollback_transaction();
-            exit_with_error('FailedToInsertCommitter', array('committer' => $committer_data));
-        }
-    }
-
-    $previous_commit_revision = array_get($commit_info, 'previousCommit');
-    $previous_commit_id = NULL;
-    if ($previous_commit_revision) {
-        $previous_commit = $db->select_first_row('commits', 'commit', array('repository' => $repository_id, 'revision' => $previous_commit_revision));
-        if (!$previous_commit) {
-            $db->rollback_transaction();
-            exit_with_error('FailedToFindPreviousCommit', array('commit' => $commit_info));
-        }
-        $previous_commit_id = $previous_commit['commit_id'];
-    }
-
-    $data = array(
-        'repository' => $repository_id,
-        'revision' => $commit_info['revision'],
-        'previous_commit' => $previous_commit_id,
-        'order' => array_get($commit_info, 'order'),
-        'time' => array_get($commit_info, 'time'),
-        'committer' => $committer_id,
-        'message' => array_get($commit_info, 'message'),
-        'reported' => true,
-    );
-    $inserted_commit_id = $db->update_or_insert_row('commits', 'commit', array('repository' => $repository_id, 'revision' => $data['revision']), $data);
-
-    if ($owner_commit_id)
-        $db->select_or_insert_row('commit_ownerships', 'commit', array('owner' => $owner_commit_id, 'owned' => $inserted_commit_id), NULL, '*');
-    return $inserted_commit_id;
-}
-
 main(file_get_contents('php://input'));
 
 ?>
index 9c76263..d5f61a6 100644 (file)
@@ -62,6 +62,7 @@ class CommitLogFetcher {
             commit_repository as "repository",
             commit_message as "message",
             commit_order as "order",
+            commit_testability as "testability",
             EXISTS(SELECT * FROM commit_ownerships WHERE commit_owner = commit_id) as "ownsCommits"
             FROM commits LEFT OUTER JOIN committers ON commit_committer = committer_id
             WHERE commit_repository = $1 AND commit_reported = true';
@@ -211,6 +212,7 @@ class CommitLogFetcher {
             'authorName' => $committer_row ? $committer_row['committer_name'] : null,
             'authorEmail' => $committer_row ? $committer_row['committer_account'] : null,
             'message' => $commit_row['commit_message'],
+            'testability' => $commit_row['commit_testability'],
             'ownsCommits' => $owns_commits
         );
     }
diff --git a/Websites/perf.webkit.org/public/include/commit-updater.php b/Websites/perf.webkit.org/public/include/commit-updater.php
new file mode 100644 (file)
index 0000000..08c68bb
--- /dev/null
@@ -0,0 +1,228 @@
+<?php
+
+class CommitUpdater {
+    private $db;
+    private $top_level_repository_id_by_name;
+    private $owned_repository_by_name_and_owner_id;
+    private $commits_by_repository_id;
+    private $authors_by_repository_id;
+
+    function __construct($db)
+    {
+        $this->db = $db;
+        $this->top_level_repository_id_by_name = array();
+        $this->owned_repository_by_name_and_owner_id = array();
+        $this->commits_by_repository_id = array();
+        $this->authors_by_repository_id = array();
+    }
+
+    function report_commits(&$commit_info_list, $should_insert)
+    {
+        $update_list = &$this->construct_update_list($commit_info_list, $should_insert);
+        $this->db->begin_transaction();
+
+        foreach ($update_list as &$update)
+            $this->update_commit($update, NULL, NULL, $should_insert);
+
+        $this->db->commit_transaction();
+    }
+
+    private function update_commit(&$update, $owner_commit_id, $owner_repository_id, $should_insert)
+    {
+        $commit_data = &$this->resolve_fields_from_database($update, $owner_repository_id, $should_insert);
+
+        $commit_select_query = array('repository' => $commit_data['repository'], 'revision' => $commit_data['revision']);
+        if ($should_insert) {
+            $commit_id = $this->db->update_or_insert_row('commits', 'commit', $commit_select_query, $commit_data);
+            if (!$commit_id)
+                $this->exit_with_error('FailedToInsertCommit', array('commit' => $commit_data));
+
+            if ($owner_commit_id)
+                $this->db->select_or_insert_row('commit_ownerships', 'commit', array('owner' => $owner_commit_id, 'owned' => $commit_id), NULL, '*');
+        } else {
+            $commit_id = $this->db->update_row('commits', 'commit', $commit_select_query, $commit_data);
+            if (!$commit_id)
+                $this->exit_with_error('FailedToUpdateCommit', array('commit' => $commit_select_query, 'commitUpdate' => $commit_data));
+
+            if ($owner_commit_id)
+                $this->exit_with_error('AttemptToUpdateOwnedCommits', array('commit' => $commit_select_query));
+        }
+
+        if (!array_key_exists('owned_commits', $update))
+            return;
+
+        foreach ($update['owned_commits'] as &$owned_commit_update)
+            $this->update_commit($owned_commit_update, $commit_id, $commit_data['repository'], $should_insert);
+    }
+
+    private function &resolve_fields_from_database(&$update, $owner_repository_id, $should_insert)
+    {
+        $commit_data = &$update['commit'];
+
+        $repository_id = $this->resolve_repository($update['repository'], $owner_repository_id, $should_insert);
+        $commit_data['repository'] = $repository_id;
+
+        if (array_key_exists('previous_commit', $update))
+            $commit_data['previous_commit'] = $this->resolve_previous_commit($repository_id, $update['previous_commit']);
+
+        if (array_key_exists('author', $update))
+            $commit_data['committer'] = $this->resolve_committer($repository_id, $update['author']);
+
+        return $commit_data;
+    }
+
+    private function &construct_update_list(&$commit_info_list, $should_insert)
+    {
+        $update_list = array();
+
+        foreach ($commit_info_list as &$commit_info) {
+            self::validate_commits($commit_info);
+            $commit_data = &self::construct_commit_data($commit_info, NULL, $should_insert);
+            $has_update = count($commit_data) > 1;
+
+            $update = array('commit' => &$commit_data, 'repository' => &$commit_info['repository']);
+            if (array_key_exists('previousCommit', $commit_info)) {
+                $has_update = true;
+                $update['previous_commit'] = &$commit_info['previousCommit'];
+            }
+
+            if (array_key_exists('author', $commit_info)) {
+                $has_update = true;
+                $update['author'] = &$commit_info['author'];
+            }
+
+            if (array_key_exists('ownedCommits', $commit_info) && !$should_insert)
+                $this->exit_with_error('AttemptToUpdateOwnedCommits', array('commit' => $commit_info));
+
+            if (!$should_insert && !$has_update)
+                $this->exit_with_error('NothingToUpdate', array('commit' => $commit_data));
+
+            if (!array_key_exists('ownedCommits', $commit_info)) {
+                array_push($update_list, $update);
+                continue;
+            }
+
+            $owned_commit_update_list = array();
+            foreach($commit_info['ownedCommits'] as $owned_repository_name => &$owned_commit_info) {
+                $owned_commit = &self::construct_commit_data($owned_commit_info, $owned_repository_name, $should_insert);
+                $owned_commit_update = array('commit' => &$owned_commit, 'repository' => $owned_repository_name);
+
+                if (array_key_exists('previousCommit', $owned_commit_info))
+                    $owned_commit_update['previous_commit'] = $owned_commit_info['previousCommit'];
+
+                if (array_key_exists('author', $owned_commit_info))
+                    $owned_commit_update['author'] = &$owned_commit_info['author'];
+                array_push($owned_commit_update_list, $owned_commit_update);
+            }
+            if (count($owned_commit_update_list))
+                $update['owned_commits'] = $owned_commit_update_list;
+
+            array_push($update_list, $update);
+        }
+
+        return $update_list;
+    }
+
+    private static function &construct_commit_data(&$commit_info, $owned_repository_name, $should_insert)
+    {
+        $commit_data = array();
+        $commit_data['revision'] = $commit_info['revision'];
+
+        if (array_key_exists('message', $commit_info))
+            $commit_data['message'] = $commit_info['message'];
+
+        if (array_key_exists('order', $commit_info))
+            $commit_data['order'] = $commit_info['order'];
+
+        if (array_key_exists('testability', $commit_info))
+            $commit_data['testability'] = $commit_info['testability'];
+
+        if (array_key_exists('time', $commit_info)) {
+            if ($owned_repository_name)
+                exit_with_error('OwnedCommitShouldNotContainTimestamp', array('commit' => $commit_info));
+            $commit_data['time'] = $commit_info['time'];
+        }
+
+        if ($should_insert)
+            $commit_data['reported'] = true;
+
+        return $commit_data;
+    }
+
+    private static function validate_commits(&$commit_info)
+    {
+        if (!array_key_exists('repository', $commit_info))
+            exit_with_error('MissingRepositoryName', array('commit' => $commit_info));
+        if (!array_key_exists('revision', $commit_info))
+            exit_with_error('MissingRevision', array('commit' => $commit_info));
+        require_format('Revision', $commit_info['revision'], '/^[A-Za-z0-9 \.]+$/');
+        if (array_key_exists('author', $commit_info) && !is_array($commit_info['author']))
+            exit_with_error('InvalidAuthorFormat', array('commit' => $commit_info));
+        if (array_key_exists('previousCommit', $commit_info))
+            require_format('Revision', $commit_info['previousCommit'], '/^[A-Za-z0-9 \.]+$/');
+    }
+
+    private function resolve_repository($repository_name, $owner_repository_id, $should_insert)
+    {
+        if ($owner_repository_id)
+            $repository_id_by_name = &array_ensure_item_has_array($this->owned_repository_by_name_and_owner_id, $owner_repository_id);
+        else
+            $repository_id_by_name = &$this->top_level_repository_id_by_name;
+
+        if (array_key_exists($repository_name, $repository_id_by_name))
+            return $repository_id_by_name[$repository_name];
+
+        if ($should_insert) {
+            $repository_id = $this->db->select_or_insert_row('repositories', 'repository', array('name' => $repository_name, 'owner' => $owner_repository_id));
+            if (!$repository_id)
+                $this->exit_with_error('FailedToInsertRepository', array('repository' => $repository_name, 'owner' => $owner_repository_id));
+        } else {
+            $repository = $this->db->select_first_row('repositories', 'repository', array('name' => $repository_name, 'owner' => $owner_repository_id));
+            if (!$repository)
+                $this->exit_with_error('InvalidRepository', array('repository' => $repository_name));
+            $repository_id = $repository['repository_id'];
+        }
+
+        $repository_id_by_name[$repository_name] = $repository_id;
+        return $repository_id;
+    }
+
+    private function resolve_previous_commit($repository_id, $revision)
+    {
+        $commit_id_by_revision = &array_ensure_item_has_array($this->commits_by_repository_id, $revision);
+        if (array_key_exists($revision, $commit_id_by_revision))
+            return $commit_id_by_revision[$revision];
+
+        $previous_commit = $this->db->select_first_row('commits', 'commit', array('repository' => $repository_id, 'revision' => $revision));
+        if (!$previous_commit)
+            $this->exit_with_error('FailedToFindPreviousCommit', array('revision' => $revision));
+
+        $commit_id_by_revision[$revision] = $previous_commit['commit_id'];
+        return $previous_commit['commit_id'];
+    }
+
+    private function resolve_committer($repository_id, $author)
+    {
+        $committer_id_by_account = &array_ensure_item_has_array($this->authors_by_repository_id, $repository_id);
+        $account = array_get($author, 'account');
+        if (array_key_exists($account, $committer_id_by_account))
+            return $committer_id_by_account[$account];
+
+        $committer_data = array('repository' => $repository_id, 'account' => $account);
+        $name = array_get($author, 'name');
+        if ($name)
+            $committer_data['name'] = $name;
+        $committer_id = $this->db->update_or_insert_row('committers', 'committer', array('repository' => $repository_id, 'account' => $account), $committer_data);
+
+        if (!$committer_id)
+            $this->exit_with_error('FailedToInsertCommitter', array('committer' => $committer_data));
+        $committer_id_by_account[$account] = $committer_id;
+        return $committer_id;
+    }
+
+    private function exit_with_error($message, $details)
+    {
+        $this->db->rollback_transaction();
+        exit_with_error($message, $details);
+    }
+}
\ No newline at end of file
index 130cb90..d9bd438 100644 (file)
@@ -29,11 +29,14 @@ class CommitLog extends DataModelObject {
             this._rawData.ownsCommits = rawData.ownsCommits;
         if (rawData.order)
             this._rawData.order = rawData.order;
+        if (rawData.testability)
+            this._rawData.testability = rawData.testability;
     }
 
     repository() { return this._repository; }
     time() { return new Date(this._rawData['time']); }
     hasCommitTime() { return this._rawData['time'] > 0 && this._rawData['time'] != null; }
+    testability() { return this._rawData['testability']; }
     author() { return this._rawData['authorName']; }
     revision() { return this._rawData['revision']; }
     message() { return this._rawData['message']; }
index 94acc53..445b270 100644 (file)
@@ -71,6 +71,8 @@ class CommitSet extends DataModelObject {
     ownerCommitForRepository(repository) { return this._repositoryToCommitOwnerMap.get(repository); }
     topLevelRepositories() { return Repository.sortByNamePreferringOnesWithURL(this._repositories.filter((repository) => !this.ownerRevisionForRepository(repository))); }
     ownedRepositoriesForOwnerRepository(repository) { return this._ownerRepositoryToOwnedRepositoriesMap.get(repository); }
+    commitsWithTestability() { return this.commits().filter((commit) => !!commit.testability()); }
+    commits() { return  Array.from(this._repositoryToCommitMap.values()); }
 
     revisionForRepository(repository)
     {
@@ -387,6 +389,9 @@ class IntermediateCommitSet {
         return Promise.all(fetchingPromises);
     }
 
+    commitsWithTestabilityWarnings() { return this.commits().filter((commit) => !!commit.testabilityWarning()); }
+    commits() { return  Array.from(this._commitByRepository.values()); }
+
     _fetchCommitLogAndOwnedCommits(repository, revision)
     {
         return CommitLog.fetchForSingleRevision(repository, revision).then((commits) => {
index f224df7..3247339 100644 (file)
@@ -6,7 +6,7 @@ const TestServer = require('./resources/test-server.js');
 const addSlaveForReport = require('./resources/common-operations.js').addSlaveForReport;
 const prepareServerTest = require('./resources/common-operations.js').prepareServerTest;
 
-describe("/api/report-commits/", function () {
+describe("/api/report-commits/ with insert=true", function () {
     prepareServerTest(this);
 
     const emptyReport = {
@@ -296,7 +296,7 @@ describe("/api/report-commits/", function () {
                 "message": "WebKit Commit",
             }
         ]
-    }
+    };
 
     it("should distinguish between repositories with the same name but with a different owner.", () => {
         return addSlaveForReport(sameRepositoryNameInOwnedCommitAndMajorCommit).then(() => {
@@ -336,7 +336,7 @@ describe("/api/report-commits/", function () {
                 }
             }
         ]
-    }
+    };
 
     it("should accept inserting one commit with some owned commits", () => {
         const db = TestServer.database();
@@ -591,3 +591,187 @@ describe("/api/report-commits/", function () {
         });
     });
 });
+
+describe("/api/report-commits/ with insert=false", function () {
+    prepareServerTest(this);
+
+    const subversionCommits = {
+        "slaveName": "someSlave",
+        "slavePassword": "somePassword",
+        "commits": [
+            {
+                "repository": "WebKit",
+                "revision": "210948",
+                "time": "2017-01-20T02:52:34.577Z",
+                "author": {"name": "Zalan Bujtas", "account": "zalan@apple.com"},
+                "message": "a message",
+            },
+            {
+                "repository": "WebKit",
+                "revision": "210949",
+                "time": "2017-01-20T03:23:50.645Z",
+                "author": {"name": "Chris Dumez", "account": "cdumez@apple.com"},
+                "message": "some message",
+            }
+        ],
+        "insert": true,
+    };
+
+    const commitsUpdate = {
+        "slaveName": "someSlave",
+        "slavePassword": "somePassword",
+        "commits": [
+            {
+                "repository": "WebKit",
+                "revision": "210948",
+                "testability": "Breaks builds",
+                "message": "another message",
+                "order": 210948,
+                "time": "2017-01-20T03:52:34.577Z",
+                "author": {"name": "Chris Dumez", "account": "cdumez@apple.com"},
+            },
+            {
+                "repository": "WebKit",
+                "revision": "210949",
+                "previousCommit": "210948",
+                "testability": "Crashes WebKit",
+                "message": "another message",
+                "order": 210949,
+                "time": "2017-01-20T04:23:50.645Z",
+                "author": {"name": "Zalan Bujtas", "account": "zalan@apple.com"},
+            }
+        ],
+        "insert": false,
+    };
+
+
+    const commitsUpdateWithMissingRevision = {
+        "slaveName": "someSlave",
+        "slavePassword": "somePassword",
+        "commits": [
+            {
+                "repository": "WebKit",
+                "testability": "Breaks builds"
+            }
+        ],
+        "insert": false,
+    };
+
+    const commitsUpdateWithMissingRepository = {
+        "slaveName": "someSlave",
+        "slavePassword": "somePassword",
+        "commits": [
+            {
+                "revision": "210948",
+                "testability": "Breaks builds"
+            }
+        ],
+        "insert": false,
+    };
+
+    const commitsUpdateWithoutAnyUpdate = {
+        "slaveName": "someSlave",
+        "slavePassword": "somePassword",
+        "commits": [
+            {
+                "repository": "WebKit",
+                "revision": "210948"
+            }
+        ],
+        "insert": false,
+    };
+
+    const commitsUpdateWithOwnedCommitsUpdate = {
+        "slaveName": "someSlave",
+        "slavePassword": "somePassword",
+        "commits": [
+            {
+                "repository": "WebKit",
+                "revision": "210948",
+                "ownedCommits": [],
+            }
+        ],
+        "insert": false,
+    };
+
+    async function initialReportCommits()
+    {
+        await addSlaveForReport(subversionCommits);
+        await TestServer.remoteAPI().postJSON('/api/report-commits/', subversionCommits);
+        const result = await TestServer.remoteAPI().getJSON('/api/commits/WebKit/');
+        assert.equal(result['status'], 'OK');
+        const commits = result['commits'];
+        assert.equal(commits.length, 2);
+        assert.equal(commits[0].testability, null);
+        assert.equal(commits[1].testability, null);
+
+        return commits;
+    }
+
+    async function setUpTestsWithExpectedStatus(commitsUpdate, expectedStatus)
+    {
+        await initialReportCommits();
+        let result = await TestServer.remoteAPI().postJSON('/api/report-commits/', commitsUpdate);
+        assert.equal(result['status'], expectedStatus);
+        result = await TestServer.remoteAPI().getJSON('/api/commits/WebKit/');
+        return result['commits'];
+    }
+
+    async function testWithExpectedFailure(commitsUpdate, expectedFailureName)
+    {
+        const commits = await setUpTestsWithExpectedStatus(commitsUpdate, expectedFailureName);
+        assert.equal(commits.length, 2);
+        assert.equal(commits[0].testability, null);
+        assert.equal(commits[1].testability, null);
+    }
+
+    it('should fail with "MissingRevision" if commit update does not have commit revision specified', async () => {
+        await testWithExpectedFailure(commitsUpdateWithMissingRevision, 'MissingRevision');
+    });
+
+    it('should fail with "MissingRepositoryName" if commit update does not have commit revision specified', async () => {
+        await testWithExpectedFailure(commitsUpdateWithMissingRepository, 'MissingRepositoryName');
+    });
+
+    it('should fail with "NothingToUpdate" if commit update does not have commit revision specified', async () => {
+        await testWithExpectedFailure(commitsUpdateWithoutAnyUpdate, 'NothingToUpdate');
+    });
+
+    it('should fail with "AttemptToUpdateOwnedCommits" when trying to update owned commits info', async () => {
+        await testWithExpectedFailure(commitsUpdateWithOwnedCommitsUpdate, 'AttemptToUpdateOwnedCommits');
+    });
+
+    it('should not set reported to true if it was not set to true before', async () => {
+        const database = TestServer.database();
+        await initialReportCommits();
+
+        let commits = await database.selectAll('commits');
+        assert.equal(commits[0].reported, true);
+        assert.equal(commits[1].reported, true);
+
+        await database.query('UPDATE commits SET commit_reported=false');
+        let result = await TestServer.remoteAPI().postJSON('/api/report-commits/', commitsUpdate);
+        assert.equal(result['status'], 'OK');
+
+        commits = await database.selectAll('commits');
+        assert.equal(commits[0].reported, false);
+        assert.equal(commits[1].reported, false);
+    });
+
+    it("should be able to update commits message, time, order, author and previous commit", async () => {
+        const commits = await setUpTestsWithExpectedStatus(commitsUpdate, 'OK');
+
+        assert.equal(commits.length, 2);
+        assert.equal(commits[0].testability, 'Breaks builds');
+        assert.equal(commits[1].testability, 'Crashes WebKit');
+        assert.equal(commits[0].message, 'another message');
+        assert.equal(commits[1].message, 'another message');
+        assert.equal(commits[0].authorName, 'Chris Dumez');
+        assert.equal(commits[1].authorName, 'Zalan Bujtas');
+        assert.equal(commits[0].order, 210948);
+        assert.equal(commits[1].order, 210949);
+        assert.equal(commits[0].time, 1484884354577);
+        assert.equal(commits[1].time, 1484886230645);
+        assert.equal(commits[1].previousCommit, commits[0].id);
+    });
+});
\ No newline at end of file
index c6f0dc9..f7a1eb6 100644 (file)
@@ -134,39 +134,57 @@ describe('OSBuildFetcher', function() {
     });
 
     describe('OSBuilderFetcher._commitsForAvailableBuilds', () => {
-        it('should only return commits whose orders are higher than specified order', () => {
+        it('should compatible with command output only contains lines of revision', async () => {
             const logger = new MockLogger;
             const fetcher = new OSBuildFetcher({}, null, null, MockSubprocess, logger);
             const waitForInvocationPromise = MockSubprocess.waitForInvocation();
-            const fetchCommitsPromise = fetcher._commitsForAvailableBuilds('OSX', ['list', 'build1'], '^\\.*$', 1604000000, 1606000000);
+            const fetchCommitsPromise = fetcher._commitsForAvailableBuilds(['list', 'build1'], '^\\.*$');
 
-            return waitForInvocationPromise.then(() => {
-                assert.equal(MockSubprocess.invocations.length, 1);
-                assert.deepEqual(MockSubprocess.invocations[0].command, ['list', 'build1']);
-                MockSubprocess.invocations[0].resolve('16D321\n16E321z\n\n16F321');
-                return fetchCommitsPromise;
-            }).then((results) => {
-                assert.equal(results.length, 2);
-                assert.deepEqual(results[0], {repository: 'OSX', order: 1604032126, revision: '16E321z'});
-                assert.deepEqual(results[1], {repository: 'OSX', order: 1605032100, revision: '16F321'});
-            });
+            await waitForInvocationPromise;
+            assert.equal(MockSubprocess.invocations.length, 1);
+            assert.deepEqual(MockSubprocess.invocations[0].command, ['list', 'build1']);
+
+            const expectedResults = {allRevisions: ["16D321", "16E321z", "16F321"], commitsWithTestability: {}};
+            await MockSubprocess.invocations[0].resolve('16D321\n16E321z\n\n16F321');
+            const buildInfo = await fetchCommitsPromise;
+            assert.deepEqual(expectedResults, buildInfo);
         });
 
-        it('should only return commits whose orders are higher than minOrder and lower than the maxOrder', () => {
+        it('should parse the command output as JSON format', async () => {
             const logger = new MockLogger;
             const fetcher = new OSBuildFetcher({}, null, null, MockSubprocess, logger);
             const waitForInvocationPromise = MockSubprocess.waitForInvocation();
-            const fetchCommitsPromise = fetcher._commitsForAvailableBuilds('OSX', ['list', 'build1'], '^\\.*$', 1604000000, 1605000000);
+            const fetchCommitsPromise = fetcher._commitsForAvailableBuilds(['list', 'build1']);
 
-            return waitForInvocationPromise.then(() => {
-                assert.equal(MockSubprocess.invocations.length, 1);
-                assert.deepEqual(MockSubprocess.invocations[0].command, ['list', 'build1']);
-                MockSubprocess.invocations[0].resolve('16D321\n16E321z\n\n16F321');
-                return fetchCommitsPromise;
-            }).then((results) => {
-                assert.equal(results.length, 1);
-                assert.deepEqual(results[0], {repository: 'OSX', order: 1604032126, revision: '16E321z'});
-            });
+            await waitForInvocationPromise;
+            assert.equal(MockSubprocess.invocations.length, 1);
+            assert.deepEqual(MockSubprocess.invocations[0].command, ['list', 'build1']);
+
+            const outputObject = {allRevisions: ["16D321", "16E321z", "16F321"], commitsWithTestability: {"16D321": "Panic"}};
+            await MockSubprocess.invocations[0].resolve(JSON.stringify(outputObject));
+            const buildInfo = await fetchCommitsPromise;
+            assert.deepEqual(outputObject, buildInfo);
+        });
+    });
+
+
+    describe('OSBuilderFetcher._commitsWithinRange', () => {
+        it('should only return commits whose orders are higher than specified order', async () => {
+            const logger = new MockLogger;
+            const fetcher = new OSBuildFetcher({}, null, null, MockSubprocess, logger);
+            const results = fetcher._commitsWithinRange(["16D321", "16E321z", "16F321"], "OSX", 1604000000, 1606000000);
+            assert.equal(results.length, 2);
+            assert.deepEqual(results[0], {repository: 'OSX', order: 1604032126, revision: '16E321z'});
+            assert.deepEqual(results[1], {repository: 'OSX', order: 1605032100, revision: '16F321'});
+
+        });
+
+        it('should only return commits whose orders are higher than minOrder and lower than the maxOrder', async () => {
+            const logger = new MockLogger;
+            const fetcher = new OSBuildFetcher({}, null, null, MockSubprocess, logger);
+            const results = fetcher._commitsWithinRange(["16D321", "16E321z", "16F321"], "OSX", 1604000000, 1605000000);
+            assert.equal(results.length, 1);
+            assert.deepEqual(results[0], {repository: 'OSX', order: 1604032126, revision: '16E321z'});
         });
     });
 
@@ -239,9 +257,9 @@ describe('OSBuildFetcher', function() {
                 });
             });
         })
-    })
+    });
 
-    describe('OSBuildFetcher.fetchAndReportNewBuilds', () => {
+    describe('OSBuildFetcher.fetchReportAndUpdateBuilds', () => {
         const invocations = MockSubprocess.invocations;
 
         beforeEach(function () {
@@ -252,11 +270,12 @@ describe('OSBuildFetcher', function() {
             TestServer.database().disconnect();
         });
 
-        it('should report all build commits with owned-commits', () => {
+        it('should be backward compatible and report all build commits with owned-commits', () => {
             const logger = new MockLogger;
             const fetcher = new OSBuildFetcher(config, TestServer.remoteAPI(), slaveAuth, MockSubprocess, logger);
             const db = TestServer.database();
-            let fetchAndReportPromise = null;
+
+            let fetchReportAndUpdateBuildsPromise = null;
             let fetchAvailableBuildsPromise = null;
 
             return addSlaveForReport(emptyReport).then(() => {
@@ -307,10 +326,9 @@ describe('OSBuildFetcher', function() {
                 assert.deepEqual(invocations[0].command, ['list', 'ownedCommit', 'for', 'revision', 'Sierra16E34']);
                 invocations[0].resolve(JSON.stringify(anotherownedCommitWithWebKitAndJavaScriptCore));
                 return fetchAvailableBuildsPromise;
-            }).then((results) => {
-                assert.equal(results.length, 3);
+            }).then(() => {
                 MockSubprocess.reset();
-                fetchAndReportPromise = fetcher.fetchAndReportNewBuilds();
+                fetchReportAndUpdateBuildsPromise = fetcher.fetchReportAndUpdateBuilds();
                 return MockSubprocess.waitForInvocation();
             }).then(() => {
                 assert.equal(invocations.length, 1);
@@ -337,9 +355,141 @@ describe('OSBuildFetcher', function() {
                 invocations[0].resolve(JSON.stringify(anotherownedCommitWithWebKitAndJavaScriptCore));
                 assert.deepEqual(invocations[0].command, ['list', 'ownedCommit', 'for', 'revision', 'Sierra16E34']);
 
-                return fetchAndReportPromise;
+                return fetchReportAndUpdateBuildsPromise;
+            }).then(() => {
+                return Promise.all([
+                    db.selectRows('repositories', {'name': 'WebKit'}),
+                    db.selectRows('repositories', {'name': 'JavaScriptCore'}),
+                    db.selectRows('commits', {'revision': 'Sierra16D69'}),
+                    db.selectRows('commits', {'revision': 'Sierra16E33h'}),
+                    db.selectRows('commits', {'revision': 'Sierra16E34'})]);
+            }).then((results) => {
+                const webkitRepository = results[0];
+                const jscRepository = results[1];
+                const osxCommit16D69 = results[2];
+                const osxCommit16E33h = results[3];
+                const osxCommit16E34 = results[4];
+
+                assert.equal(webkitRepository.length, 1);
+                assert.equal(webkitRepository[0]['owner'], 10);
+                assert.equal(jscRepository.length, 1);
+                assert.equal(jscRepository[0]['owner'], 10);
+
+                assert.equal(osxCommit16D69.length, 1);
+                assert.equal(osxCommit16D69[0]['repository'], 10);
+                assert.equal(osxCommit16D69[0]['order'], 1603006900);
+
+                assert.equal(osxCommit16E33h.length, 1);
+                assert.equal(osxCommit16E33h[0]['repository'], 10);
+                assert.equal(osxCommit16E33h[0]['order'], 1604003308);
+
+                assert.equal(osxCommit16E34.length, 1);
+                assert.equal(osxCommit16E34[0]['repository'], 10);
+                assert.equal(osxCommit16E34[0]['order'], 1604003400);
+
+                return TestServer.remoteAPI().getJSON('/api/commits/OSX/last-reported?from=1603000000&to=1603099900');
             }).then((result) => {
-                assert.equal(result['status'], 'OK');
+                assert.equal(result['commits'].length, 1);
+                assert.equal(result['commits'][0]['revision'], 'Sierra16D69');
+                assert.equal(result['commits'][0]['order'], 1603006900);
+
+                return TestServer.remoteAPI().getJSON('/api/commits/OSX/last-reported?from=1604000000&to=1604099900');
+            }).then((result) => {
+                assert.equal(result['commits'].length, 1);
+                assert.equal(result['commits'][0]['revision'], 'Sierra16E34');
+                assert.equal(result['commits'][0]['order'], 1604003400);
+            });
+        });
+
+        it('should report all build commits with owned-commits', () => {
+            const logger = new MockLogger;
+            const fetcher = new OSBuildFetcher(config, TestServer.remoteAPI(), slaveAuth, MockSubprocess, logger);
+            const db = TestServer.database();
+            const resultsForSierraD = {allRevisions: ["Sierra16D68", "Sierra16D69"], commitsWithTestability: {}};
+            const resultsForSierraE = {allRevisions: ["Sierra16E32", "Sierra16E33", "Sierra16E33h", "Sierra16E34"], commitsWithTestability: {}};
+
+            let fetchReportAndUpdateBuildsPromise = null;
+            let fetchAvailableBuildsPromise = null;
+
+            return addSlaveForReport(emptyReport).then(() => {
+                return Promise.all([
+                    db.insert('repositories', {'id': 10, 'name': 'OSX'}),
+                    db.insert('commits', {'repository': 10, 'revision': 'Sierra16D67', 'order': 1603006700, 'reported': true}),
+                    db.insert('commits', {'repository': 10, 'revision': 'Sierra16D68', 'order': 1603006800, 'reported': true}),
+                    db.insert('commits', {'repository': 10, 'revision': 'Sierra16D69', 'order': 1603006900, 'reported': false}),
+                    db.insert('commits', {'repository': 10, 'revision': 'Sierra16E32', 'order': 1604003200, 'reported': true}),
+                    db.insert('commits', {'repository': 10, 'revision': 'Sierra16E33', 'order': 1604003300, 'reported': true}),
+                    db.insert('commits', {'repository': 10, 'revision': 'Sierra16E33g', 'order': 1604003307, 'reported': true})]);
+            }).then(() => {
+                return TestServer.remoteAPI().getJSON('/api/commits/OSX/last-reported?from=1603000000&to=1603099900');
+            }).then((result) => {
+                assert.equal(result['commits'].length, 1);
+                assert.equal(result['commits'][0]['revision'], 'Sierra16D68');
+                assert.equal(result['commits'][0]['order'], 1603006800);
+                return TestServer.remoteAPI().getJSON('/api/commits/OSX/last-reported?from=1604000000&to=1604099900');
+            }).then((result) => {
+                assert.equal(result['commits'].length, 1);
+                assert.equal(result['commits'][0]['revision'], 'Sierra16E33g');
+                assert.equal(result['commits'][0]['order'], 1604003307);
+                const waitForInvocationPromise = MockSubprocess.waitForInvocation();
+                fetchAvailableBuildsPromise = fetcher._fetchAvailableBuilds();
+                return waitForInvocationPromise;
+            }).then(() => {
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'all osx 16Dxx builds']);
+                invocations[0].resolve(JSON.stringify(resultsForSierraD));
+                return MockSubprocess.resetAndWaitForInvocation();
+            }).then(() => {
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'ownedCommit', 'for', 'revision', 'Sierra16D69']);
+                invocations[0].resolve(JSON.stringify(ownedCommitWithWebKit));
+                return MockSubprocess.resetAndWaitForInvocation();
+            }).then(() => {
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'all osx 16Exx builds']);
+                invocations[0].resolve(JSON.stringify(resultsForSierraE));
+                return MockSubprocess.resetAndWaitForInvocation();
+            }).then(() => {
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'ownedCommit', 'for', 'revision', 'Sierra16E33h']);
+                invocations[0].resolve(JSON.stringify(anotherownedCommitWithWebKit));
+                return MockSubprocess.resetAndWaitForInvocation();
+            }).then(() => {
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'ownedCommit', 'for', 'revision', 'Sierra16E34']);
+                invocations[0].resolve(JSON.stringify(anotherownedCommitWithWebKitAndJavaScriptCore));
+                return fetchAvailableBuildsPromise;
+            }).then(() => {
+                MockSubprocess.reset();
+                fetchReportAndUpdateBuildsPromise = fetcher.fetchReportAndUpdateBuilds();
+                return MockSubprocess.waitForInvocation();
+            }).then(() => {
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'all osx 16Dxx builds']);
+                invocations[0].resolve(JSON.stringify(resultsForSierraD));
+                return MockSubprocess.resetAndWaitForInvocation();
+            }).then(() => {
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'ownedCommit', 'for', 'revision', 'Sierra16D69']);
+                invocations[0].resolve(JSON.stringify(ownedCommitWithWebKit));
+                return MockSubprocess.resetAndWaitForInvocation();
+            }).then(() => {
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'all osx 16Exx builds']);
+                invocations[0].resolve(JSON.stringify(resultsForSierraE));
+                return MockSubprocess.resetAndWaitForInvocation();
+            }).then(() => {
+                assert.equal(invocations.length, 1);
+                assert.deepEqual(invocations[0].command, ['list', 'ownedCommit', 'for', 'revision', 'Sierra16E33h']);
+                invocations[0].resolve(JSON.stringify(anotherownedCommitWithWebKit));
+                return MockSubprocess.resetAndWaitForInvocation();
+            }).then(() => {
+                assert.equal(invocations.length, 1);
+                invocations[0].resolve(JSON.stringify(anotherownedCommitWithWebKitAndJavaScriptCore));
+                assert.deepEqual(invocations[0].command, ['list', 'ownedCommit', 'for', 'revision', 'Sierra16E34']);
+
+                return fetchReportAndUpdateBuildsPromise;
+            }).then(() => {
                 return Promise.all([
                     db.selectRows('repositories', {'name': 'WebKit'}),
                     db.selectRows('repositories', {'name': 'JavaScriptCore'}),
@@ -384,11 +534,119 @@ describe('OSBuildFetcher', function() {
             });
         });
 
+        it('should update testability warning for commits', async () => {
+            const logger = new MockLogger;
+            const fetcher = new OSBuildFetcher(config, TestServer.remoteAPI(), slaveAuth, MockSubprocess, logger);
+            const db = TestServer.database();
+            const resultsForSierraD = {allRevisions: ["Sierra16D68", "Sierra16D69"], commitsWithTestability: {"Sierra16D68": "Panic", "Sierra16D69": "Spin CPU"}};
+            const resultsForSierraE = {allRevisions: ["Sierra16E32", "Sierra16E33", "Sierra16E33h", "Sierra16E34"], commitsWithTestability: {"Sierra16E31": "WebKit crashes"}};
+
+            await addSlaveForReport(emptyReport);
+
+            await Promise.all([
+                db.insert('repositories', {'id': 10, 'name': 'OSX'}),
+                db.insert('commits', {'repository': 10, 'revision': 'Sierra16D67', 'order': 1603006700, 'reported': true}),
+                db.insert('commits', {'repository': 10, 'revision': 'Sierra16D68', 'order': 1603006800, 'reported': true}),
+                db.insert('commits', {'repository': 10, 'revision': 'Sierra16D69', 'order': 1603006900, 'reported': false}),
+                db.insert('commits', {'repository': 10, 'revision': 'Sierra16E31', 'order': 1604003100, 'reported': true}),
+                db.insert('commits', {'repository': 10, 'revision': 'Sierra16E32', 'order': 1604003200, 'reported': true}),
+                db.insert('commits', {'repository': 10, 'revision': 'Sierra16E33', 'order': 1604003300, 'reported': true}),
+                db.insert('commits', {'repository': 10, 'revision': 'Sierra16E33g', 'order': 1604003307, 'reported': true})]);
+
+            let result = await TestServer.remoteAPI().getJSON('/api/commits/OSX/last-reported?from=1603000000&to=1603099900');
+
+            assert.equal(result['commits'].length, 1);
+            assert.equal(result['commits'][0]['revision'], 'Sierra16D68');
+            assert.equal(result['commits'][0]['order'], 1603006800);
+            result = await TestServer.remoteAPI().getJSON('/api/commits/OSX/last-reported?from=1604000000&to=1604099900');
+
+            assert.equal(result['commits'].length, 1);
+            assert.equal(result['commits'][0]['revision'], 'Sierra16E33g');
+            assert.equal(result['commits'][0]['order'], 1604003307);
+
+            const fetchReportAndUpdatePromise = fetcher.fetchReportAndUpdateBuilds();
+            await MockSubprocess.waitForInvocation();
+
+            assert.equal(invocations.length, 1);
+            assert.deepEqual(invocations[0].command, ['list', 'all osx 16Dxx builds']);
+            invocations[0].resolve(JSON.stringify(resultsForSierraD));
+            await MockSubprocess.resetAndWaitForInvocation();
+
+            assert.equal(invocations.length, 1);
+            assert.deepEqual(invocations[0].command, ['list', 'ownedCommit', 'for', 'revision', 'Sierra16D69']);
+            invocations[0].resolve(JSON.stringify(ownedCommitWithWebKit));
+            await MockSubprocess.resetAndWaitForInvocation();
+
+            assert.equal(invocations.length, 1);
+            assert.deepEqual(invocations[0].command, ['list', 'all osx 16Exx builds']);
+            invocations[0].resolve(JSON.stringify(resultsForSierraE));
+
+            await MockSubprocess.resetAndWaitForInvocation();
+
+            assert.equal(invocations.length, 1);
+            assert.deepEqual(invocations[0].command, ['list', 'ownedCommit', 'for', 'revision', 'Sierra16E33h']);
+            invocations[0].resolve(JSON.stringify(anotherownedCommitWithWebKit));
+            await  MockSubprocess.resetAndWaitForInvocation();
+            assert.equal(invocations.length, 1);
+            invocations[0].resolve(JSON.stringify(anotherownedCommitWithWebKitAndJavaScriptCore));
+            assert.deepEqual(invocations[0].command, ['list', 'ownedCommit', 'for', 'revision', 'Sierra16E34']);
+
+            await fetchReportAndUpdatePromise;
+
+            const webkitRepository = await db.selectRows('repositories', {'name': 'WebKit'});
+            const jscRepository = await db.selectRows('repositories', {'name': 'JavaScriptCore'});
+            const osxCommit16D68 = await db.selectRows('commits', {'revision': 'Sierra16D68'});
+            const osxCommit16D69 = await db.selectRows('commits', {'revision': 'Sierra16D69'});
+            const osxCommit16E31 = await db.selectRows('commits', {'revision': 'Sierra16E31'});
+            const osxCommit16E33h = await db.selectRows('commits', {'revision': 'Sierra16E33h'});
+            const osxCommit16E34 = await db.selectRows('commits', {'revision': 'Sierra16E34'});
+
+            assert.equal(webkitRepository.length, 1);
+            assert.equal(webkitRepository[0]['owner'], 10);
+            assert.equal(jscRepository.length, 1);
+            assert.equal(jscRepository[0]['owner'], 10);
+
+            assert.equal(osxCommit16D68.length, 1);
+            assert.equal(osxCommit16D68[0]['repository'], 10);
+            assert.equal(osxCommit16D68[0]['order'], 1603006800);
+            assert.equal(osxCommit16D68[0]['testability'], "Panic");
+
+            assert.equal(osxCommit16D69.length, 1);
+            assert.equal(osxCommit16D69[0]['repository'], 10);
+            assert.equal(osxCommit16D69[0]['order'], 1603006900);
+            assert.equal(osxCommit16D69[0]['testability'], "Spin CPU");
+
+            assert.equal(osxCommit16E31.length, 1);
+            assert.equal(osxCommit16E31[0]['repository'], 10);
+            assert.equal(osxCommit16E31[0]['order'], 1604003100);
+            assert.equal(osxCommit16E31[0]['testability'], "WebKit crashes");
+
+            assert.equal(osxCommit16E33h.length, 1);
+            assert.equal(osxCommit16E33h[0]['repository'], 10);
+            assert.equal(osxCommit16E33h[0]['order'], 1604003308);
+
+            assert.equal(osxCommit16E34.length, 1);
+            assert.equal(osxCommit16E34[0]['repository'], 10);
+            assert.equal(osxCommit16E34[0]['order'], 1604003400);
+
+            result = await TestServer.remoteAPI().getJSON('/api/commits/OSX/last-reported?from=1603000000&to=1603099900');
+            assert.equal(result['commits'].length, 1);
+            assert.equal(result['commits'][0]['revision'], 'Sierra16D69');
+            assert.equal(result['commits'][0]['order'], 1603006900);
+
+            result = await TestServer.remoteAPI().getJSON('/api/commits/OSX/last-reported?from=1604000000&to=1604099900');
+            assert.equal(result['commits'].length, 1);
+            assert.equal(result['commits'][0]['revision'], 'Sierra16E34');
+            assert.equal(result['commits'][0]['order'], 1604003400);
+        });
+
         it('should report commits without owned-commits if "ownedCommitCommand" is not specified in config', async () => {
 
             const logger = new MockLogger;
             const fetcher = new OSBuildFetcher(configWithoutOwnedCommitCommand, TestServer.remoteAPI(), slaveAuth, MockSubprocess, logger);
             const db = TestServer.database();
+            const resultsForSierraD = {allRevisions: ["Sierra16D68", "Sierra16D69"], commitsWithTestability: {}};
+            const resultsForSierraE = {allRevisions: ["Sierra16E32", "Sierra16E33", "Sierra16E33h", "Sierra16E34"], commitsWithTestability: {}};
 
             await addSlaveForReport(emptyReport);
             await Promise.all([
@@ -411,19 +669,18 @@ describe('OSBuildFetcher', function() {
             assert.equal(result['commits'][0]['order'], 1604003307);
 
             const waitForInvocationPromise = MockSubprocess.waitForInvocation();
-            const fetchAndReportPromise = fetcher.fetchAndReportNewBuilds();
+            const fetchReportAndUpdatePromise = fetcher.fetchReportAndUpdateBuilds();
             await waitForInvocationPromise;
             assert.equal(invocations.length, 1);
             assert.deepEqual(invocations[0].command, ['list', 'all osx 16Dxx builds']);
-            invocations[0].resolve('\n\nSierra16D68\nSierra16D69');
+            invocations[0].resolve(JSON.stringify(resultsForSierraD));
 
             await MockSubprocess.resetAndWaitForInvocation();
             assert.equal(invocations.length, 1);
             assert.deepEqual(invocations[0].command, ['list', 'all osx 16Exx builds']);
-            invocations[0].resolve('\n\nSierra16E32\nSierra16E33\nSierra16E33h\nSierra16E34');
+            invocations[0].resolve(JSON.stringify(resultsForSierraE));
 
-            result = await fetchAndReportPromise;
-            assert.equal(result['status'], 'OK');
+            result = await fetchReportAndUpdatePromise;
             const results = await Promise.all([
                 db.selectRows('repositories', {'name': 'WebKit'}),
                 db.selectRows('repositories', {'name': 'JavaScriptCore'}),
@@ -467,6 +724,8 @@ describe('OSBuildFetcher', function() {
             const logger = new MockLogger;
             const fetcher = new OSBuildFetcher(configWithoutOwnedCommitCommand, TestServer.remoteAPI(), slaveAuth, MockSubprocess, logger);
             const db = TestServer.database();
+            const resultsForSierraD = {allRevisions: ["Sierra16D68", "Sierra16D69", "Sierra16D10000"], commitsWithTestability: {}};
+            const resultsForSierraE = {allRevisions: ["Sierra16E32", "Sierra16E33", "Sierra16E33h", "Sierra16E34", "Sierra16E10000"], commitsWithTestability: {}};
 
             await addSlaveForReport(emptyReport);
             await Promise.all([
@@ -488,20 +747,19 @@ describe('OSBuildFetcher', function() {
             assert.equal(result['commits'][0]['revision'], 'Sierra16E33g');
             assert.equal(result['commits'][0]['order'], 1604003307);
             const waitForInvocationPromise = MockSubprocess.waitForInvocation();
-            const fetchAndReportPromise = fetcher.fetchAndReportNewBuilds();
+            const fetchReportAndUpdatePromise = fetcher.fetchReportAndUpdateBuilds();
 
             await waitForInvocationPromise;
             assert.equal(invocations.length, 1);
             assert.deepEqual(invocations[0].command, ['list', 'all osx 16Dxx builds']);
-            invocations[0].resolve('\n\nSierra16D68\nSierra16D69\nSierra16D10000');
+            invocations[0].resolve(JSON.stringify(resultsForSierraD));
 
             await MockSubprocess.resetAndWaitForInvocation();
             assert.equal(invocations.length, 1);
             assert.deepEqual(invocations[0].command, ['list', 'all osx 16Exx builds']);
-            invocations[0].resolve('\n\nSierra16E32\nSierra16E33\nSierra16E33h\nSierra16E34\nSierra16E10000');
+            invocations[0].resolve(JSON.stringify(resultsForSierraE));
 
-            result = await fetchAndReportPromise;
-            assert.equal(result['status'], 'OK');
+            result = await fetchReportAndUpdatePromise;
             const results = await Promise.all([
                 db.selectRows('repositories', {'name': 'WebKit'}),
                 db.selectRows('repositories', {'name': 'JavaScriptCore'}),
@@ -545,6 +803,7 @@ describe('OSBuildFetcher', function() {
             const logger = new MockLogger;
             const fetcher = new OSBuildFetcher(configTrackingOneOS, TestServer.remoteAPI(), slaveAuth, MockSubprocess, logger);
             const db = TestServer.database();
+            const resultsForSierraD = {allRevisions: ["Sierra16D68", "Sierra16D69", "Sierra16D100", "Sierra16D100a"], commitsWithTestability: {}};
 
             await addSlaveForReport(emptyReport);
             await db.insert('repositories', {'id': 10, 'name': 'OSX'});
@@ -556,14 +815,13 @@ describe('OSBuildFetcher', function() {
             assert.equal(result['commits'][0]['order'], 1603010000);
 
             const waitForInvocationPromise = MockSubprocess.waitForInvocation();
-            const fetchAndReportPromise = fetcher.fetchAndReportNewBuilds();
+            const fetchAndReportPromise = fetcher.fetchReportAndUpdateBuilds();
             await waitForInvocationPromise;
             assert.equal(invocations.length, 1);
             assert.deepEqual(invocations[0].command, ['list', 'all osx 16Dxx builds']);
-            invocations[0].resolve('\n\nSierra16D68\nSierra16D69\nSierra16D100\nSierra16D100a\n');
+            invocations[0].resolve(JSON.stringify(resultsForSierraD));
 
             result = await fetchAndReportPromise;
-            assert.equal(result['status'], 'OK');
             const results = await Promise.all([
                 db.selectRows('repositories', {'name': 'WebKit'}),
                 db.selectRows('repositories', {'name': 'JavaScriptCore'}),
@@ -594,6 +852,7 @@ describe('OSBuildFetcher', function() {
             const logger = new MockLogger;
             const fetcher = new OSBuildFetcher(configTrackingOneOS, TestServer.remoteAPI(), slaveAuth, MockSubprocess, logger);
             const db = TestServer.database();
+            const resultsForSierraD = {allRevisions: ["Sierra16D68", "Sierra16D69", "Sierra16D100", "Sierra16D101"], commitsWithTestability: {}};
 
             await addSlaveForReport(emptyReport);
             await db.insert('repositories', {'id': 10, 'name': 'OSX'});
@@ -602,14 +861,13 @@ describe('OSBuildFetcher', function() {
             assert.equal(result['commits'].length, 0);
 
             const waitForInvocationPromise = MockSubprocess.waitForInvocation();
-            const fetchAndReportPromise = fetcher.fetchAndReportNewBuilds();
+            const fetchReportAndUpdatePromise = fetcher.fetchReportAndUpdateBuilds();
             await waitForInvocationPromise;
             assert.equal(invocations.length, 1);
             assert.deepEqual(invocations[0].command, ['list', 'all osx 16Dxx builds']);
-            invocations[0].resolve('\n\nSierra16D68\nSierra16D69\nSierra16D100\nSierra16D101\n');
+            invocations[0].resolve(JSON.stringify(resultsForSierraD));
 
-            result = await fetchAndReportPromise;
-            assert.equal(result['status'], 'OK');
+            result = await fetchReportAndUpdatePromise;
             const results = await Promise.all([
                 db.selectRows('repositories', {'name': 'WebKit'}),
                 db.selectRows('repositories', {'name': 'JavaScriptCore'}),
@@ -646,6 +904,8 @@ describe('OSBuildFetcher', function() {
             const logger = new MockLogger;
             const fetcher = new OSBuildFetcher(config, TestServer.remoteAPI(), slaveAuth, MockSubprocess, logger);
             const db = TestServer.database();
+            const resultsForSierraD = {allRevisions: ["Sierra16D68", "Sierra16D69"], commitsWithTestability: {}};
+            const resultsForSierraE = {allRevisions: ["Sierra16E32", "Sierra16E33", "Sierra16E33h", "Sierra16E34"], commitsWithTestability: {}};
             let fetchAndReportPromise = null;
 
             return addSlaveForReport(emptyReport).then(() => {
@@ -671,12 +931,12 @@ describe('OSBuildFetcher', function() {
                 assert.equal(result['commits'][0]['order'], 1604003307);
 
                 const waitForInvocationPromise = MockSubprocess.waitForInvocation();
-                fetchAndReportPromise = fetcher.fetchAndReportNewBuilds();
+                fetchAndReportPromise = fetcher.fetchReportAndUpdateBuilds();
                 return waitForInvocationPromise;
             }).then(() => {
                 assert.equal(invocations.length, 1);
                 assert.deepEqual(invocations[0].command, ['list', 'all osx 16Dxx builds']);
-                invocations[0].resolve('\n\nSierra16D68\nSierra16D69\n');
+                invocations[0].resolve(JSON.stringify(resultsForSierraD));
                 return MockSubprocess.resetAndWaitForInvocation();
             }).then(() => {
                 assert.equal(invocations.length, 1);
@@ -686,7 +946,7 @@ describe('OSBuildFetcher', function() {
             }).then(() => {
                 assert.equal(invocations.length, 1);
                 assert.deepEqual(invocations[0].command, ['list', 'all osx 16Exx builds']);
-                invocations[0].resolve('\n\nSierra16E32\nSierra16E33\nSierra16E33h\nSierra16E34');
+                invocations[0].resolve(JSON.stringify(resultsForSierraE));
                 return MockSubprocess.resetAndWaitForInvocation();
             }).then(() => {
                 assert.deepEqual(invocations[0].command, ['list', 'ownedCommit', 'for', 'revision', 'Sierra16E33h']);
index 051c29f..0806325 100644 (file)
@@ -22,59 +22,56 @@ class OSBuildFetcher {
         this._maxSubmitCount = osConfig['maxSubmitCount'] || 20;
     }
 
-    static fetchAndReportAllInOrder(fetcherList)
+    static async fetchReportAndUpdateCommits(fetcherList)
     {
-        return mapInSerialPromiseChain(fetcherList, (fetcher) => fetcher.fetchAndReportNewBuilds());
+        for (const fetcher of fetcherList)
+            await fetcher.fetchReportAndUpdateBuilds();
     }
 
-    fetchAndReportNewBuilds()
+    async fetchReportAndUpdateBuilds()
     {
-        return this._fetchAvailableBuilds().then((results) => {
-            this._logger.log(`Submitting ${results.length} builds for ${this._osConfig['name']}`);
-            if (results.length == 0)
-                return this._submitCommits(results);
-
-            const splittedResults = [];
-            for (let startIndex = 0; startIndex < results.length; startIndex += this._maxSubmitCount)
-                splittedResults.push(results.slice(startIndex, startIndex + this._maxSubmitCount));
-
-            return mapInSerialPromiseChain(splittedResults, this._submitCommits.bind(this)).then((responses) => {
-                assert(responses.every((response) => response['status'] == 'OK'));
-                assert(responses.length > 0);
-                return responses[0];
-            });
-        });
+        const {newCommitsToReport, commitsToUpdate} = await this._fetchAvailableBuilds();
+
+        this._logger.log(`Submitting ${newCommitsToReport.length} builds for ${this._osConfig['name']}`);
+        await this._reportCommits(newCommitsToReport, true);
+
+        this._logger.log(`Updating ${commitsToUpdate.length} builds for ${this._osConfig['name']}`);
+        await this._reportCommits(commitsToUpdate, false);
     }
 
-    _fetchAvailableBuilds()
+    async _fetchAvailableBuilds()
     {
         const config = this._osConfig;
         const repositoryName = config['name'];
-        let customCommands = config['customCommands'];
+        const newCommitsToReport = [];
+        const commitsToUpdate = [];
+        const  customCommands = config['customCommands'];
 
-        return mapInSerialPromiseChain(customCommands, (command) => {
+        for (const command of customCommands) {
             assert(command['minRevision']);
             assert(command['maxRevision']);
             const minRevisionOrder = this._computeOrder(command['minRevision']);
             const maxRevisionOrder = this._computeOrder(command['maxRevision']);
 
             const url = `/api/commits/${escape(repositoryName)}/last-reported?from=${minRevisionOrder}&to=${maxRevisionOrder}`;
+            const result = await this._remoteAPI.getJSONWithStatus(url);
+            const minOrder = result['commits'].length == 1 ? parseInt(result['commits'][0]['order']) + 1 : minRevisionOrder;
 
-            return this._remoteAPI.getJSONWithStatus(url).then((result) => {
-                const minOrder = result['commits'].length == 1 ? parseInt(result['commits'][0]['order']) + 1 : minRevisionOrder;
-                return this._commitsForAvailableBuilds(repositoryName, command['command'], command['linesToIgnore'], minOrder, maxRevisionOrder);
-            }).then((commits) => {
-                const label = 'name' in command ? `"${command['name']}"` : `"${command['minRevision']}" to "${command['maxRevision']}"`;
-                this._logger.log(`Found ${commits.length} builds for ${label}`);
+            const commitInfo = await this._commitsForAvailableBuilds(command['command'], command['linesToIgnore']);
+            const commits = this._commitsWithinRange(commitInfo.allRevisions, repositoryName, minOrder, maxRevisionOrder);
 
-                if ('ownedCommitCommand' in command) {
-                    this._logger.log(`Resolving ownedCommits for ${label}`);
-                    return this._addOwnedCommitsForBuild(commits, command['ownedCommitCommand']);
-                }
+            const label = 'name' in command ? `"${command['name']}"` : `"${command['minRevision']}" to "${command['maxRevision']}"`;
+            this._logger.log(`Found ${commits.length} builds for ${label}`);
+            if ('ownedCommitCommand' in command) {
+                this._logger.log(`Resolving ownedCommits for ${label}`);
+                await this._addOwnedCommitsForBuild(commits, command['ownedCommitCommand']);
+            }
+            newCommitsToReport.push(...commits);
 
-                return commits;
-            });
-        }).then((results) => [].concat(...results));
+            for (const [revision, testability] of Object.entries(commitInfo.commitsWithTestability))
+                commitsToUpdate.push({repository: repositoryName, revision, testability});
+        }
+        return {newCommitsToReport, commitsToUpdate};
     }
 
     _computeOrder(revision)
@@ -89,17 +86,27 @@ class OSBuildFetcher {
         return ((major * 100 + kind) * 10000 + minor) * 100 + variant;
     }
 
-    _commitsForAvailableBuilds(repository, command, linesToIgnore, minOrder, maxOrder)
+    async _commitsForAvailableBuilds(command, linesToIgnore)
     {
-        return this._subprocess.execute(command).then((output) => {
+        const output = await this._subprocess.execute(command);
+        try {
+            return JSON.parse(output);
+        } catch (error) {
+            if (!(error instanceof SyntaxError))
+                throw error;
             let lines = output.split('\n');
-            if (linesToIgnore){
+            if (linesToIgnore) {
                 const regex = new RegExp(linesToIgnore);
                 lines = lines.filter((line) => !regex.exec(line));
             }
-            return lines.map((revision) => ({repository, revision, 'order': this._computeOrder(revision)}))
-                .filter((commit) => commit['order'] >= minOrder && commit['order'] <= maxOrder);
-        });
+            return {allRevisions: lines, commitsWithTestability: {}};
+        }
+    }
+
+    _commitsWithinRange(revisions, repository, minOrder, maxOrder)
+    {
+        return revisions.map((revision) => ({repository, revision, 'order': this._computeOrder(revision)}))
+            .filter((commit) => commit['order'] >= minOrder && commit['order'] <= maxOrder);
     }
 
     _addOwnedCommitsForBuild(commits, command)
@@ -119,10 +126,16 @@ class OSBuildFetcher {
         });
     }
 
-    _submitCommits(commits)
+    async _reportCommits(commitsToPost, insert)
     {
-        const commitsToReport = {"slaveName": this._slaveAuth['name'], "slavePassword": this._slaveAuth['password'], 'commits': commits};
-        return this._remoteAPI.postJSONWithStatus('/api/report-commits/', commitsToReport);
+        if (!commitsToPost.length)
+            return;
+
+        for(let commitsToSubmit = commitsToPost.splice(0, this._maxSubmitCount); commitsToSubmit.length; commitsToSubmit = commitsToPost.splice(0, this._maxSubmitCount)) {
+            const report = {"slaveName": this._slaveAuth['name'], "slavePassword": this._slaveAuth['password'], 'commits': commitsToSubmit, insert};
+            const response = await this._remoteAPI.postJSONWithStatus('/api/report-commits/', report);
+            assert(response['status'] === 'OK');
+        }
     }
 }
 
index 77f4a0a..4fbe582 100644 (file)
@@ -29,7 +29,7 @@ function syncLoop(options)
     const remoteAPI = new RemoteAPI(serverConfig.server);
 
     const fetchers = osConfigList.map((osConfig) => new OSBuildFetcher(osConfig, remoteAPI, serverConfig.slave, new Subprocess, console));
-    OSBuildFetcher.fetchAndReportAllInOrder(fetchers).catch((error) => {
+    OSBuildFetcher.fetchReportAndUpdateCommits(fetchers).catch((error) => {
         console.error(error);
         if (typeof(error.stack) == 'string') {
             for (let line of error.stack.split('\n'))
index 5f88524..86a1263 100644 (file)
@@ -58,6 +58,18 @@ function osxCommit()
     });
 }
 
+function osxCommitWithTestability()
+{
+    return new CommitLog(5, {
+        id: 5,
+        repository: MockModels.osx,
+        revision: '10.11.4 15E65',
+        time: null,
+        order: 1504065,
+        testability: "Panic on boot"
+    });
+}
+
 function oldOSXCommit()
 {
     return new CommitLog(6, {
@@ -417,6 +429,13 @@ describe('CommitLog', function () {
         });
     });
 
+    describe('testability', () => {
+        it('should return "testability" message', () => {
+            const commit = osxCommitWithTestability();
+            assert.equal(commit.testability(), 'Panic on boot');
+        });
+    });
+
     const commitsAPIResponse = {
         "commits":[{
             "id": "831151",
index 39fccc5..7d52ea5 100644 (file)
@@ -170,6 +170,18 @@ function anotherCommitWithGitRevision()
     });
 }
 
+function commitWithTestability()
+{
+    return CommitLog.ensureSingleton(2023, {
+        id: 2023,
+        repository: MockModels.webkitGit,
+        revision: 'f24fc67873a0068fd5a3a9adac595f0036c21315',
+        ownsCommits: false,
+        time: 1456932773000,
+        testability: 'Failed compilation'
+    });
+}
+
 describe('CommitSet', () => {
     MockRemoteAPI.inject(null, BrowserPrivilegedAPI);
     MockModels.inject();
@@ -294,6 +306,15 @@ describe('CommitSet', () => {
         });
     }
 
+    function commitSetWithTestability()
+    {
+        return CommitSet.ensureSingleton(16, {
+            revisionItems: [{ commit: commitWithTestability(), requiresBuild: false },
+                { commit: webkitCommit(), requiresBuild: false }],
+            customRoots: []
+        });
+    }
+
     function oneMeasurementCommitSet()
     {
         return MeasurementCommitSet.ensureSingleton(1, [
@@ -406,6 +427,25 @@ describe('CommitSet', () => {
                     {'11': { revision: 'webkit-commit-0', ownerRevision: null, patch: null}, customRoots: [456, 458]}]);
         });
     });
+
+    describe('commitsWithTestability', () => {
+        it('should return commits with testability', () => {
+            const commits = commitSetWithTestability().commitsWithTestability();
+            assert.equal(commits.length, 1);
+            assert.equal(commits[0], commitWithTestability());
+        });
+
+        it('should return empty list if no commit in commit set contains testability', () => {
+            const commits = commitSetWithTwoCommits().commitsWithTestability();
+            assert.equal(commits.length, 0);
+        });
+    });
+
+    describe('commits', () => {
+        it('should return all commits in commit set', () => {
+            assert.equal(commitSetWithTwoCommits().commits().length, 2);
+        });
+    });
 });
 
 describe('IntermediateCommitSet', () => {
@@ -629,9 +669,7 @@ describe('IntermediateCommitSet', () => {
                 assert(!commitSet.commitForRepository(MockModels.webkit));
             });
         });
-
     });
-
 });
 
 describe('CustomCommitSet', () => {