Add the support for perf try bots to sync-buildbot.js
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 May 2017 18:47:01 +0000 (18:47 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 May 2017 18:47:01 +0000 (18:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=172529

Rubber-stamped by Chris Dumez.

Make sync-buildbot.js schedule an A/B testing job with a patch or roots to buildbot.

Change the buildbot property format in the syncing script's configuration again to use a dictionary
with a single key of "revision", "patch", or "roots" to specify a revision, a patch, or a set of roots,
and simplified the structure of the configuration by always having "types" and "builders", and
make each entry in "configurations" refer to a list of types, platforms, and builders.

Since now there are build requests to build patches and run tests, "configurations" has been renamed to
"testConfigurations" and "buildConfigurations" have been added. Each entry in "buildConfigurations"
specifies a list of platforms and builders. Similarly in repository group configurations, the buildbot
properties for testing is now specified as "testProperties" and ones for building a patch is specified
in newly introduced "buildProperties".

* public/api/build-requests.php:
(update_builds): When a build request to build a patch fails, mark all subsequent requests as failed
since there is no way to run tests without a successful build.

* public/api/update-triggerable.php:
(main): Re-generate manifest.json after updating the triggerable. The lack of this re-generation was
the reason we had to manually GET /api/manifest in api-update-triggerable-tests.js.

* public/v3/models/build-request.js:
(BuildRequest.prototype.hasCompleted): Added.

* public/v3/models/manifest.js:
(Manifest.reset): Added. Extracted from MockData.resetV3Models in unit-tests/mock-data.js and
syncLoop in tools/sync-buildbot.js
(Manifest.fetch): Reset V3 models before fetching the manifest. This eliminates the need to manually
reset V3 models in syncLoop.

* public/v3/models/uploaded-file.js:
(UploadedFile.prototype.url): Use RemoteAPI.url to get the full URL instead of just a path.

* public/v3/remote.js:
(BrowserRemoteAPI.prototype.url): Added. Constructs the full URL.

* server-tests/api-update-triggerable-tests.js:
(.refetchManifest): Deleted. Now that /api/manifest re-generates manifest.json, we can simply call
Manifest.fetch instead.

* server-tests/resources/mock-data.js:
(MockData.resetV3Models): Calls Manifest.reset().
(MockData.addMockConfiguration): Extracted from addMockData.
(MockData.addMockData): Updated per the format change.
(MockData.mockTestSyncConfigWithSingleBuilder): Ditto.
(MockData.mockTestSyncConfigWithTwoBuilders): Ditto.
(MockData.runningBuild): Make buildNumber specifiable.
(MockData.finishedBuild): Ditto.

* server-tests/tools-buildbot-triggerable-tests.js: Updated configurations per the format change.
Now that now acceptsCustomRoots() for "system-and-webkit" must be true since we can't have a
repository group that which accepts a patch and not take roots.

* server-tests/tools-sync-buildbot-integration-tests.js: Added.
(createTriggerable): Added.
(createTestGroupWihPatch): Added.
(uploadRoot): Added.
(.assertAndResolveRequest): Added.
(.assertTestBuildHasFailed): Added.

* tools/js/buildbot-syncer.js:
(BuildbotSyncer): Added. _type as an instance variable to identify whether this buildbot builder
is a "builder" which builds a patch, builder, or a "tester" which runs a test. Also renamed
_testConfigurations to _configurations.
(BuildbotSyncer.prototype.addTestConfiguration): Assert that either the type of this syncer hasn't
been set or it's a tester.
(BuildbotSyncer.prototype.testConfigurations): Return [] when it's a builder.
(BuildbotSyncer.prototype.addBuildConfiguration): Added. Adds a platform to a builder.
(BuildbotSyncer.prototype.buildConfigurations): Added. Returns the list of configurations if this
syncer is a builder. Otherwise returns [].
(BuildbotSyncer.prototype.isTester): Added.
(BuildbotSyncer.prototype.matchesConfiguration):
(BuildbotSyncer.prototype._propertiesForBuildRequest): Updated to support the new format.
(BuildbotSyncer._loadConfig): Ditto. Optionally parse buildConfigurations.
(BuildbotSyncer._resolveBuildersWithPlatforms): Added. For each test or build configuration entry,
creates the list of configurations per builder and platform.
(BuildbotSyncer._parseRepositoryGroup): Added the support for parsing the new format with revision,
roots, and patch option types with a lot of validations as we're seeing a bit of combinatorial
explosion in the number of things that can go wrong. Also parse buildProperties optionally.
(BuildbotSyncer._parseRepositoryGroupPropertyTemplate): Added. A helper function to parse a set of
buildbot properties, validates its content, and invokes a callback if it's an dynamically resolved
type such as "revision" and "patch".
(BuildbotSyncer._validateAndMergeConfig): Updated per the format change. No longer allows "types",
"type", "platforms", and "platform" as they're explicity resolved in _resolveBuildersWithPlatforms.

* tools/js/buildbot-triggerable.js:
(BuildbotTriggerable.prototype.syncOnce):
(BuildbotTriggerable.prototype._validateRequests): Handle the case when a build request is not
associated with any test.
(BuildbotTriggerable.prototype._nextRequestInGroup): Return null when there is a build request to
build a patch which has not been completed (pending, scheduled, running, or failed). Since all
requests to build a patch has a negative order, those requests should all show up at the beginning.
(BuildbotTriggerable.prototype._scheduleRequestIfSlaveIsAvailable): Pick a new buildbot syncer when
scheduling the first request to build a patch or the first request to run a test. The first request
to run a test will always have order of 0, so it's a sufficient condition to find such a request.
On the other hand, the first request to build a patch can have a negative order number so we must
explicitly check if it's the first item in the ordered list of requests in the test group.

* tools/remote-server-relay.log: Added.

* tools/sync-buildbot.js:
(syncLoop): Fixed a bug we were not re-fetching the triggerable after updating the triggerable so
that Triggerable and related objects we have in the memory may not reflect what we just synced to
the perf dashboard. Also, we don't reset V3 models manually any more since Manifest.fetch does that.

* unit-tests/buildbot-syncer-tests.js: Added more test cases and updated existing test cases to test
exception messages explicitly since allowing any exception was resulting in some tests passing a
result of unrelated parsing error being thrown, etc...
(sampleiOSConfig): Updated per the format change.
(sampleiOSConfigWithExpansions): Ditto.
(smallConfiguration): Ditto.

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

15 files changed:
Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/public/api/build-requests.php
Websites/perf.webkit.org/public/api/update-triggerable.php
Websites/perf.webkit.org/public/v3/models/build-request.js
Websites/perf.webkit.org/public/v3/models/manifest.js
Websites/perf.webkit.org/public/v3/models/uploaded-file.js
Websites/perf.webkit.org/public/v3/remote.js
Websites/perf.webkit.org/server-tests/api-update-triggerable-tests.js
Websites/perf.webkit.org/server-tests/resources/mock-data.js
Websites/perf.webkit.org/server-tests/tools-buildbot-triggerable-tests.js
Websites/perf.webkit.org/server-tests/tools-sync-buildbot-integration-tests.js [new file with mode: 0644]
Websites/perf.webkit.org/tools/js/buildbot-syncer.js
Websites/perf.webkit.org/tools/js/buildbot-triggerable.js
Websites/perf.webkit.org/tools/sync-buildbot.js
Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js

index 039ce59..82ce8ee 100644 (file)
@@ -1,3 +1,122 @@
+2017-05-23  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Add the support for perf try bots to sync-buildbot.js
+        https://bugs.webkit.org/show_bug.cgi?id=172529
+
+        Rubber-stamped by Chris Dumez.
+
+        Make sync-buildbot.js schedule an A/B testing job with a patch or roots to buildbot.
+
+        Change the buildbot property format in the syncing script's configuration again to use a dictionary
+        with a single key of "revision", "patch", or "roots" to specify a revision, a patch, or a set of roots,
+        and simplified the structure of the configuration by always having "types" and "builders", and
+        make each entry in "configurations" refer to a list of types, platforms, and builders.
+
+        Since now there are build requests to build patches and run tests, "configurations" has been renamed to
+        "testConfigurations" and "buildConfigurations" have been added. Each entry in "buildConfigurations"
+        specifies a list of platforms and builders. Similarly in repository group configurations, the buildbot
+        properties for testing is now specified as "testProperties" and ones for building a patch is specified
+        in newly introduced "buildProperties".
+
+        * public/api/build-requests.php:
+        (update_builds): When a build request to build a patch fails, mark all subsequent requests as failed
+        since there is no way to run tests without a successful build.
+
+        * public/api/update-triggerable.php:
+        (main): Re-generate manifest.json after updating the triggerable. The lack of this re-generation was
+        the reason we had to manually GET /api/manifest in api-update-triggerable-tests.js.
+
+        * public/v3/models/build-request.js:
+        (BuildRequest.prototype.hasCompleted): Added.
+
+        * public/v3/models/manifest.js:
+        (Manifest.reset): Added. Extracted from MockData.resetV3Models in unit-tests/mock-data.js and
+        syncLoop in tools/sync-buildbot.js
+        (Manifest.fetch): Reset V3 models before fetching the manifest. This eliminates the need to manually
+        reset V3 models in syncLoop.
+
+        * public/v3/models/uploaded-file.js:
+        (UploadedFile.prototype.url): Use RemoteAPI.url to get the full URL instead of just a path.
+
+        * public/v3/remote.js:
+        (BrowserRemoteAPI.prototype.url): Added. Constructs the full URL.
+
+        * server-tests/api-update-triggerable-tests.js:
+        (.refetchManifest): Deleted. Now that /api/manifest re-generates manifest.json, we can simply call
+        Manifest.fetch instead.
+
+        * server-tests/resources/mock-data.js:
+        (MockData.resetV3Models): Calls Manifest.reset().
+        (MockData.addMockConfiguration): Extracted from addMockData.
+        (MockData.addMockData): Updated per the format change.
+        (MockData.mockTestSyncConfigWithSingleBuilder): Ditto.
+        (MockData.mockTestSyncConfigWithTwoBuilders): Ditto.
+        (MockData.runningBuild): Make buildNumber specifiable.
+        (MockData.finishedBuild): Ditto.
+
+        * server-tests/tools-buildbot-triggerable-tests.js: Updated configurations per the format change.
+        Now that now acceptsCustomRoots() for "system-and-webkit" must be true since we can't have a
+        repository group that which accepts a patch and not take roots.
+
+        * server-tests/tools-sync-buildbot-integration-tests.js: Added.
+        (createTriggerable): Added.
+        (createTestGroupWihPatch): Added.
+        (uploadRoot): Added.
+        (.assertAndResolveRequest): Added.
+        (.assertTestBuildHasFailed): Added.
+
+        * tools/js/buildbot-syncer.js:
+        (BuildbotSyncer): Added. _type as an instance variable to identify whether this buildbot builder
+        is a "builder" which builds a patch, builder, or a "tester" which runs a test. Also renamed
+        _testConfigurations to _configurations.
+        (BuildbotSyncer.prototype.addTestConfiguration): Assert that either the type of this syncer hasn't
+        been set or it's a tester.
+        (BuildbotSyncer.prototype.testConfigurations): Return [] when it's a builder.
+        (BuildbotSyncer.prototype.addBuildConfiguration): Added. Adds a platform to a builder.
+        (BuildbotSyncer.prototype.buildConfigurations): Added. Returns the list of configurations if this
+        syncer is a builder. Otherwise returns [].
+        (BuildbotSyncer.prototype.isTester): Added.
+        (BuildbotSyncer.prototype.matchesConfiguration):
+        (BuildbotSyncer.prototype._propertiesForBuildRequest): Updated to support the new format.
+        (BuildbotSyncer._loadConfig): Ditto. Optionally parse buildConfigurations.
+        (BuildbotSyncer._resolveBuildersWithPlatforms): Added. For each test or build configuration entry,
+        creates the list of configurations per builder and platform.
+        (BuildbotSyncer._parseRepositoryGroup): Added the support for parsing the new format with revision,
+        roots, and patch option types with a lot of validations as we're seeing a bit of combinatorial
+        explosion in the number of things that can go wrong. Also parse buildProperties optionally.
+        (BuildbotSyncer._parseRepositoryGroupPropertyTemplate): Added. A helper function to parse a set of
+        buildbot properties, validates its content, and invokes a callback if it's an dynamically resolved
+        type such as "revision" and "patch".
+        (BuildbotSyncer._validateAndMergeConfig): Updated per the format change. No longer allows "types",
+        "type", "platforms", and "platform" as they're explicity resolved in _resolveBuildersWithPlatforms.
+
+        * tools/js/buildbot-triggerable.js:
+        (BuildbotTriggerable.prototype.syncOnce):
+        (BuildbotTriggerable.prototype._validateRequests): Handle the case when a build request is not
+        associated with any test.
+        (BuildbotTriggerable.prototype._nextRequestInGroup): Return null when there is a build request to
+        build a patch which has not been completed (pending, scheduled, running, or failed). Since all
+        requests to build a patch has a negative order, those requests should all show up at the beginning.
+        (BuildbotTriggerable.prototype._scheduleRequestIfSlaveIsAvailable): Pick a new buildbot syncer when
+        scheduling the first request to build a patch or the first request to run a test. The first request
+        to run a test will always have order of 0, so it's a sufficient condition to find such a request.
+        On the other hand, the first request to build a patch can have a negative order number so we must
+        explicitly check if it's the first item in the ordered list of requests in the test group.
+
+        * tools/remote-server-relay.log: Added.
+
+        * tools/sync-buildbot.js:
+        (syncLoop): Fixed a bug we were not re-fetching the triggerable after updating the triggerable so
+        that Triggerable and related objects we have in the memory may not reflect what we just synced to
+        the perf dashboard. Also, we don't reset V3 models manually any more since Manifest.fetch does that.
+
+        * unit-tests/buildbot-syncer-tests.js: Added more test cases and updated existing test cases to test
+        exception messages explicitly since allowing any exception was resulting in some tests passing a
+        result of unrelated parsing error being thrown, etc...
+        (sampleiOSConfig): Updated per the format change.
+        (sampleiOSConfigWithExpansions): Ditto.
+        (smallConfiguration): Ditto.
+
 2017-05-22  Dewei Zhu  <dewei_zhu@apple.com>
 
         Fix the bug that sometimes analysis task results pane is missing.
index 76b9c86..a5273fa 100644 (file)
@@ -46,8 +46,20 @@ function update_builds($db, $updates) {
         $status = $info['status'];
         $url = array_get($info, 'url');
         if ($status == 'failedIfNotCompleted') {
-            $db->query_and_get_affected_rows('UPDATE build_requests SET (request_status, request_url) = ($1, $2)
-                WHERE request_id = $3 AND request_status != $4', array('failed', $url, $id, 'completed'));
+            $request_row = $db->select_first_row('build_requests', 'request', array('id' => $id));
+            if (!$request_row) {
+                $db->rollback_transaction();
+                exit_with_error('FailedToFindBuildRequest', array('buildRequest' => $id));
+            }
+            if ($request_row['request_status'] == 'completed')
+                continue;
+            $is_build = $request_row['request_order'] < 0;
+            if ($is_build) {
+                $db->query_and_fetch_all('UPDATE build_requests SET request_status = \'failed\'
+                    WHERE request_group = $1 AND request_order > $2',
+                    array($request_row['request_group'], $request_row['request_order']));
+            }
+            $db->update_row('build_requests', 'request', array('id' => $id), array('status' => 'failed', 'url' => $url));
         } else {
             if (!in_array($status, array('pending', 'scheduled', 'running', 'failed', 'completed'))) {
                 $db->rollback_transaction();
index f5b22b5..d8f93e7 100644 (file)
@@ -1,6 +1,7 @@
 <?php
 
 require_once('../include/json-header.php');
+require_once('../include/manifest-generator.php');
 require_once('../include/repository-group-finder.php');
 
 function main($post_data)
@@ -78,6 +79,14 @@ function main($post_data)
     }
 
     $db->commit_transaction();
+
+    $generator = new ManifestGenerator($db);
+    if (!$generator->generate())
+        exit_with_error('FailedToGenerateManifest');
+
+    if (!$generator->store())
+        exit_with_error('FailedToStoreManifest');
+
     exit_with_success();
 }
 
index 97daac0..32ad431 100644 (file)
@@ -52,6 +52,7 @@ class BuildRequest extends DataModelObject {
 
     status() { return this._status; }
     hasFinished() { return this._status == 'failed' || this._status == 'completed' || this._status == 'canceled'; }
+    hasCompleted() { return this._status == 'completed'; }
     hasStarted() { return this._status != 'pending'; }
     isScheduled() { return this._status == 'scheduled'; }
     isPending() { return this._status == 'pending'; }
index 1651de0..af597e6 100644 (file)
@@ -2,8 +2,26 @@
 
 class Manifest {
 
+    static reset()
+    {
+        AnalysisTask._fetchAllPromise = null;
+        AnalysisTask.clearStaticMap();
+        BuildRequest.clearStaticMap();
+        CommitLog.clearStaticMap();
+        Metric.clearStaticMap();
+        Platform.clearStaticMap();
+        Repository.clearStaticMap();
+        CommitSet.clearStaticMap();
+        Test.clearStaticMap();
+        TestGroup.clearStaticMap();
+        Triggerable.clearStaticMap();
+        TriggerableRepositoryGroup.clearStaticMap();
+        UploadedFile.clearStaticMap();
+    }
+
     static fetch()
     {
+        this.reset();
         return RemoteAPI.getJSON('/data/manifest.json').catch(function () {
             return RemoteAPI.getJSON('/api/manifest/');
         }).then(this._didFetchManifest.bind(this));
index d7ab4ac..4fdf347 100644 (file)
@@ -19,7 +19,7 @@ class UploadedFile extends DataModelObject {
     author() { return this._author; }
     size() { return this._size; }
     label() { return this.filename(); }
-    url() { return `/api/uploaded-file/${this.id()}`; }
+    url() { return RemoteAPI.url(`/api/uploaded-file/${this.id()}`); }
 
     static uploadFile(file, uploadProgressCallback = null)
     {
index 6a00f18..513c807 100644 (file)
@@ -2,6 +2,13 @@
 
 class BrowserRemoteAPI extends CommonRemoteAPI {
 
+    url(path)
+    {
+        if (path.charAt(0) != '/')
+            path = '/' + path;
+        return `${location.protocol}//${location.host}${path}`;
+    }
+
     sendHttpRequest(path, method, contentType, content, options = {})
     {
         console.assert(!path.startsWith('http:') && !path.startsWith('https:') && !path.startsWith('file:'));
index 2b46f3d..fff2acf 100644 (file)
@@ -330,12 +330,6 @@ describe('/api/update-triggerable/', function () {
         return map;
     }
 
-    function refetchManifest()
-    {
-        MockData.resetV3Models();
-        return TestServer.remoteAPI().getJSON('/api/manifest').then((content) => Manifest._didFetchManifest(content));
-    }
-
     it('should update the acceptable of custom roots and patches', () => {
         const db = TestServer.database();
         const initialUpdate = updateWithMacWebKitRepositoryGroups();
@@ -346,7 +340,7 @@ describe('/api/update-triggerable/', function () {
             return addSlaveForReport(initialUpdate);
         }).then(() => {
             return TestServer.remoteAPI().postJSONWithStatus('/api/update-triggerable/', initialUpdate);
-        }).then(() => refetchManifest()).then(() => {
+        }).then(() => Manifest.fetch()).then(() => {
             const repositoryGroups = TriggerableRepositoryGroup.sortByName(TriggerableRepositoryGroup.all());
             const webkit = Repository.findTopLevelByName('WebKit');
             const macos = Repository.findTopLevelByName('macOS');
@@ -365,7 +359,7 @@ describe('/api/update-triggerable/', function () {
             assert.equal(repositoryGroups[1].acceptsPatchForRepository(webkit), false);
             assert.equal(repositoryGroups[1].acceptsPatchForRepository(macos), false);
             return TestServer.remoteAPI().postJSONWithStatus('/api/update-triggerable/', secondUpdate);
-        }).then(() => refetchManifest()).then(() => {
+        }).then(() => Manifest.fetch()).then(() => {
             const repositoryGroups = TriggerableRepositoryGroup.sortByName(TriggerableRepositoryGroup.all());
             const webkit = Repository.findTopLevelByName('WebKit');
             const macos = Repository.findTopLevelByName('macOS');
index 307c8f8..e1a69af 100644 (file)
@@ -5,18 +5,7 @@ var crypto = require('crypto');
 MockData = {
     resetV3Models: function ()
     {
-        AnalysisTask._fetchAllPromise = null;
-        AnalysisTask.clearStaticMap();
-        BuildRequest.clearStaticMap();
-        CommitLog.clearStaticMap();
-        Metric.clearStaticMap();
-        Platform.clearStaticMap();
-        Repository.clearStaticMap();
-        CommitSet.clearStaticMap();
-        Test.clearStaticMap();
-        TestGroup.clearStaticMap();
-        Triggerable.clearStaticMap();
-        TriggerableRepositoryGroup.clearStaticMap();
+        Manifest.reset();
     },
     emptyTriggeragbleId() { return 1001; },
     someTestId() { return 200; },
@@ -26,10 +15,8 @@ MockData = {
     webkitRepositoryId() { return 11; },
     gitWebkitRepositoryId() { return 111; },
     sharedRepositoryId() { return 14; },
-    addMockData: function (db, statusList)
+    addMockConfiguration: function (db)
     {
-        if (!statusList)
-            statusList = ['pending', 'pending', 'pending', 'pending'];
         return Promise.all([
             db.insert('build_triggerables', {id: 1000, name: 'build-webkit'}),
             db.insert('build_slaves', {id: 20, name: 'sync-slave', password_hash: crypto.createHash('sha256').update('password').digest('hex')}),
@@ -52,6 +39,14 @@ MockData = {
             db.insert('test_configurations', {id: 301, metric: 300, platform: MockData.somePlatformId(), type: 'current'}),
             db.insert('test_configurations', {id: 302, metric: 300, platform: MockData.otherPlatformId(), type: 'current'}),
             db.insert('test_runs', {id: 801, config: 301, build: 901, mean_cache: 100}),
+        ]);
+    },
+    addMockData: function (db, statusList)
+    {
+        if (!statusList)
+            statusList = ['pending', 'pending', 'pending', 'pending'];
+        return Promise.all([
+            this.addMockConfiguration(db),
             db.insert('commit_sets', {id: 401}),
             db.insert('commit_set_items', {set: 401, commit: 87832}),
             db.insert('commit_set_items', {set: 401, commit: 93116}),
@@ -128,17 +123,23 @@ MockData = {
             'repositoryGroups': {
                 'webkit-svn': {
                     'repositories': {'WebKit': {}, 'macOS': {}},
-                    'properties': {
-                        'os': '<macOS>',
-                        'wk': '<WebKit>',
+                    'testProperties': {
+                        'os': {'revision': 'macOS'},
+                        'wk': {'revision': 'WebKit'},
                     }
                 }
             },
-            'configurations': [
+            'types': {
+                'some-test': {'test': ['some test']}
+            },
+            'builders': {
+                'builder-1': {'builder': 'some-builder-1'},
+            },
+            'testConfigurations': [
                 {
-                    'platform': 'some platform',
-                    'test': ['some test'],
-                    'builder': 'some-builder-1',
+                    'platforms': ['some platform'],
+                    'types': ['some-test'],
+                    'builders': ['builder-1'],
                 }
             ]
         }
@@ -152,22 +153,24 @@ MockData = {
             'repositoryGroups': {
                 'webkit-svn': {
                     'repositories': {'WebKit': {}, 'macOS': {}},
-                    'properties': {
-                        'os': '<macOS>',
-                        'wk': '<WebKit>',
+                    'testProperties': {
+                        'os': {'revision': 'macOS'},
+                        'wk': {'revision': 'WebKit'},
                     }
                 }
             },
-            'configurations': [
-                {
-                    'platform': 'some platform',
-                    'test': ['some test'],
-                    'builder': 'some-builder-1',
-                },
+            'types': {
+                'some-test': {'test': ['some test']},
+            },
+            'builders': {
+                'builder-1': {'builder': 'some-builder-1'},
+                'builder-2': {'builder': 'some builder 2'},
+            },
+            'testConfigurations': [
                 {
-                    'platform': 'some platform',
-                    'test': ['some test'],
-                    'builder': 'some builder 2',
+                    'platforms': ['some platform'],
+                    'types': ['some-test'],
+                    'builders': ['builder-1', 'builder-2'],
                 }
             ]
         }
@@ -207,7 +210,7 @@ MockData = {
             ],
             'currentStep': {},
             'eta': 721,
-            'number': 124,
+            'number': options.buildNumber || 124,
             'source': {
                 'branch': '',
                 'changes': [],
@@ -232,7 +235,7 @@ MockData = {
             ],
             'currentStep': null,
             'eta': null,
-            'number': 123,
+            'number': options.buildNumber || 123,
             'source': {
                 'branch': '',
                 'changes': [],
index a83d652..d15b3cc 100644 (file)
@@ -951,8 +951,28 @@ describe('BuildbotTriggerable', function () {
 
                 const config = MockData.mockTestSyncConfigWithSingleBuilder();
                 config.repositoryGroups = {
-                    'system-and-roots': {description: 'Custom Roots', repositories: {'macOS': {}}, properties: {'os': '<macOS>'}, acceptsRoots: true},
-                    'system-and-webkit': {repositories: {'WebKit': {acceptsPatch: true}, 'macOS': {}}, properties: {'os': '<macOS>', 'wk': '<WebKit>'}}
+                    'system-and-roots': {
+                        description: 'Custom Roots',
+                        repositories: {'macOS': {}},
+                        testProperties: {
+                            'os': {'revision': 'macOS'},
+                            'roots': {'roots': {}}
+                        },
+                        acceptsRoots: true
+                    },
+                    'system-and-webkit': {
+                        repositories: {'WebKit': {'acceptsPatch': true}, 'macOS': {}},
+                        testProperties: {
+                            'os': {'revision': 'macOS'},
+                            'wk': {'revision': 'WebKit'},
+                            'roots': {'roots': {}},
+                        },
+                        buildProperties: {
+                            'wk': {'revision': 'WebKit'},
+                            'wk-patch': {'patch': 'WebKit'},
+                        },
+                        acceptsRoots: true
+                    }
                 }
 
                 const logger = new MockLogger;
@@ -975,7 +995,7 @@ describe('BuildbotTriggerable', function () {
                 assert.equal(groups[0].acceptsCustomRoots(), true);
                 assert.equal(groups[1].name(), 'system-and-webkit');
                 assert.deepEqual(groups[1].repositories(), [webkit, macos]);
-                assert.equal(groups[1].acceptsCustomRoots(), false);
+                assert.equal(groups[1].acceptsCustomRoots(), true);
 
                 const config = MockData.mockTestSyncConfigWithSingleBuilder();
                 config.repositoryGroups = [ ];
diff --git a/Websites/perf.webkit.org/server-tests/tools-sync-buildbot-integration-tests.js b/Websites/perf.webkit.org/server-tests/tools-sync-buildbot-integration-tests.js
new file mode 100644 (file)
index 0000000..86768f5
--- /dev/null
@@ -0,0 +1,644 @@
+'use strict';
+
+const assert = require('assert');
+
+require('../tools/js/v3-models.js');
+
+const BuildbotTriggerable = require('../tools/js/buildbot-triggerable.js').BuildbotTriggerable;
+const MockData = require('./resources/mock-data.js');
+const MockLogger = require('./resources/mock-logger.js').MockLogger;
+const MockRemoteAPI = require('../unit-tests/resources/mock-remote-api.js').MockRemoteAPI;
+const TestServer = require('./resources/test-server.js');
+const TemporaryFile = require('./resources/temporary-file.js').TemporaryFile;
+const addSlaveForReport = require('./resources/common-operations.js').addSlaveForReport;
+const prepareServerTest = require('./resources/common-operations.js').prepareServerTest;
+
+function createTriggerable()
+{
+    let triggerable;
+    const config = {
+            triggerableName: 'build-webkit',
+            lookbackCount: 2,
+            buildRequestArgument: 'build-request-id',
+            slaveName: 'sync-slave',
+            slavePassword: 'password',
+            repositoryGroups: {
+                'webkit': {
+                    repositories: {'WebKit': {acceptsPatch: true}},
+                    testProperties: {'wk': {'revision': 'WebKit'}, 'roots': {'roots': {}}},
+                    buildProperties: {'wk': {'revision': 'WebKit'}, 'wk-patch': {'patch': 'WebKit'}},
+                    acceptsRoots: true,
+                }
+            },
+            types: {
+                'some': {
+                    test: ['some test'],
+                    properties: {'test': 'some-test'},
+                }
+            },
+            builders: {
+                'builder-1': {
+                    builder: 'some tester',
+                    properties: {forcescheduler: 'force-ab-tests'},
+                },
+                'builder-2': {
+                    builder: 'some builder',
+                    properties: {forcescheduler: 'force-ab-builds'},
+                },
+                'builder-3': {
+                    builder: 'other builder',
+                    properties: {forcescheduler: 'force-ab-builds'},
+                },
+            },
+            buildConfigurations: [
+                {platforms: ['some platform'], builders: ['builder-2', 'builder-3']},
+            ],
+            testConfigurations: [
+                {types: ['some'], platforms: ['some platform'], builders: ['builder-1']},
+            ],
+        };
+    return MockData.addMockConfiguration(TestServer.database()).then(() => {
+        return Manifest.fetch();
+    }).then(() => {
+        triggerable = new BuildbotTriggerable(config, TestServer.remoteAPI(), MockRemoteAPI, {name: 'sync-slave', password: 'password'}, new MockLogger);
+        return triggerable.updateTriggerable();
+    }).then(() => Manifest.fetch()).then(() => {
+        return new BuildbotTriggerable(config, TestServer.remoteAPI(), MockRemoteAPI, {name: 'sync-slave', password: 'password'}, new MockLogger);
+    });
+}
+
+function createTestGroupWihPatch()
+{
+    return TemporaryFile.makeTemporaryFile('patch.dat', 'patch file').then((patchFile) => {
+        return UploadedFile.uploadFile(patchFile);
+    }).then((patchFile) => {
+        const someTest = Test.findById(MockData.someTestId());
+        const webkit = Repository.findById(MockData.webkitRepositoryId());
+        const set1 = new CustomCommitSet;
+        set1.setRevisionForRepository(webkit, '191622', patchFile);
+        const set2 = new CustomCommitSet;
+        set2.setRevisionForRepository(webkit, '191622');
+        return TestGroup.createWithTask('custom task', Platform.findById(MockData.somePlatformId()), someTest, 'some group', 2, [set1, set2]);
+    }).then((task) => {
+        return TestGroup.findAllByTask(task.id())[0];
+    })
+}
+
+function uploadRoot(buildRequestId, buildNumber)
+{
+    return TemporaryFile.makeTemporaryFile(`root${buildNumber}.dat`, `root for build ${buildNumber}`).then((rootFile) => {
+        return TestServer.remoteAPI().postFormData('/api/upload-root/', {
+            slaveName: 'sync-slave',
+            slavePassword: 'password',
+            builderName: 'some builder',
+            buildNumber: buildNumber,
+            buildTime: '2017-05-10T02:54:08.666',
+            buildRequest: buildRequestId,
+            rootFile,
+            repositoryList: '["WebKit"]',
+        });
+    });
+}
+
+describe('sync-buildbot', function () {
+    prepareServerTest(this);
+    TemporaryFile.inject();
+
+    beforeEach(() => {
+        MockRemoteAPI.reset('http://build.webkit.org');
+    });
+
+    function assertAndResolveRequest(request, method, url, contentToResolve)
+    {
+        assert.equal(request.method, method);
+        assert.equal(request.url, url);
+        request.resolve(contentToResolve);
+    }
+
+    it('should schedule a build to build a patch', () => {
+        const requests = MockRemoteAPI.requests;
+        let triggerable;
+        let taskId = null;
+        let syncPromise;
+        return createTriggerable().then((newTriggerable) => {
+            triggerable = newTriggerable;
+            return createTestGroupWihPatch();
+        }).then((testGroup) => {
+            taskId = testGroup.task().id();
+            const webkit = Repository.findById(MockData.webkitRepositoryId());
+            assert.equal(testGroup.buildRequests().length, 6);
+
+            const buildRequest = testGroup.buildRequests()[0];
+            assert(buildRequest.isBuild());
+            assert(!buildRequest.isTest());
+            assert.equal(buildRequest.statusLabel(), 'Waiting');
+            assert.equal(buildRequest.statusUrl(), null);
+            assert.equal(buildRequest.buildId(), null);
+
+            const commitSet = buildRequest.commitSet();
+            assert.equal(commitSet.revisionForRepository(webkit), '191622');
+            const webkitPatch = commitSet.patchForRepository(webkit);
+            assert(webkitPatch instanceof UploadedFile);
+            assert.equal(webkitPatch.filename(), 'patch.dat');
+            assert.equal(commitSet.rootForRepository(webkit), null);
+            assert.deepEqual(commitSet.allRootFiles(), []);
+
+            const otherBuildRequest = testGroup.buildRequests()[1];
+            assert(otherBuildRequest.isBuild());
+            assert(!otherBuildRequest.isTest());
+            assert.equal(otherBuildRequest.statusLabel(), 'Waiting');
+            assert.equal(otherBuildRequest.statusUrl(), null);
+            assert.equal(otherBuildRequest.buildId(), null);
+
+            const otherCommitSet = otherBuildRequest.commitSet();
+            assert.equal(otherCommitSet.revisionForRepository(webkit), '191622');
+            assert.equal(otherCommitSet.patchForRepository(webkit), null);
+            assert.equal(otherCommitSet.rootForRepository(webkit), null);
+            assert.deepEqual(otherCommitSet.allRootFiles(), []);
+
+            syncPromise = triggerable.syncOnce();
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 3);
+            assertAndResolveRequest(requests[0], 'GET', '/json/builders/some%20tester/pendingBuilds', []);
+            assertAndResolveRequest(requests[1], 'GET', '/json/builders/some%20builder/pendingBuilds', []);
+            assertAndResolveRequest(requests[2], 'GET', '/json/builders/other%20builder/pendingBuilds', []);
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 6);
+            assertAndResolveRequest(requests[3], 'GET', '/json/builders/some%20tester/builds/?select=-1&select=-2', {});
+            assertAndResolveRequest(requests[4], 'GET', '/json/builders/some%20builder/builds/?select=-1&select=-2', {});
+            assertAndResolveRequest(requests[5], 'GET', '/json/builders/other%20builder/builds/?select=-1&select=-2', {});
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 7);
+            assertAndResolveRequest(requests[6], 'POST', '/builders/some%20builder/force', 'OK');
+            assert.deepEqual(requests[6].data, {'wk': '191622', 'wk-patch': RemoteAPI.url('/api/uploaded-file/1'),
+                'build-request-id': '1', 'forcescheduler': 'force-ab-builds'});
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 10);
+            assertAndResolveRequest(requests[7], 'GET', '/json/builders/some%20tester/pendingBuilds', []);
+            assertAndResolveRequest(requests[8], 'GET', '/json/builders/some%20builder/pendingBuilds',
+                [MockData.pendingBuild({builder: 'some builder', buildRequestId: 1})]);
+            assertAndResolveRequest(requests[9], 'GET', '/json/builders/other%20builder/pendingBuilds', []);
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 13);
+            assertAndResolveRequest(requests[10], 'GET', '/json/builders/some%20tester/builds/?select=-1&select=-2', {});
+            assertAndResolveRequest(requests[11], 'GET', '/json/builders/some%20builder/builds/?select=-1&select=-2', {
+                [-1]: MockData.runningBuild({builder: 'some builder', buildRequestId: 1})
+            });
+            assertAndResolveRequest(requests[12], 'GET', '/json/builders/other%20builder/builds/?select=-1&select=-2', {});
+            return syncPromise;
+        }).then(() => {
+            return TestGroup.fetchForTask(taskId, true);
+        }).then((testGroups) => {
+            assert.equal(testGroups.length, 1);
+            const testGroup = testGroups[0];
+            const webkit = Repository.findById(MockData.webkitRepositoryId());
+            assert.equal(testGroup.buildRequests().length, 6);
+
+            const buildRequest = testGroup.buildRequests()[0];
+            assert(buildRequest.isBuild());
+            assert(!buildRequest.isTest());
+            assert.equal(buildRequest.statusLabel(), 'Running');
+            assert.equal(buildRequest.statusUrl(), 'http://build.webkit.org/builders/some%20builder/builds/124');
+            assert.equal(buildRequest.buildId(), null);
+
+            const commitSet = buildRequest.commitSet();
+            assert.equal(commitSet.revisionForRepository(webkit), '191622');
+            const webkitPatch = commitSet.patchForRepository(webkit);
+            assert(webkitPatch instanceof UploadedFile);
+            assert.equal(webkitPatch.filename(), 'patch.dat');
+            assert.equal(commitSet.rootForRepository(webkit), null);
+            assert.deepEqual(commitSet.allRootFiles(), []);
+
+            const otherBuildRequest = testGroup.buildRequests()[1];
+            assert(otherBuildRequest.isBuild());
+            assert(!otherBuildRequest.isTest());
+            assert.equal(otherBuildRequest.statusLabel(), 'Waiting');
+            assert.equal(otherBuildRequest.statusUrl(), null);
+            assert.equal(otherBuildRequest.buildId(), null);
+
+            const otherCommitSet = otherBuildRequest.commitSet();
+            assert.equal(otherCommitSet.revisionForRepository(webkit), '191622');
+            assert.equal(otherCommitSet.patchForRepository(webkit), null);
+            assert.equal(otherCommitSet.rootForRepository(webkit), null);
+            assert.deepEqual(otherCommitSet.allRootFiles(), []);
+
+            return uploadRoot(buildRequest.id(), 123);
+        }).then(() => {
+            return TestGroup.fetchForTask(taskId, true);
+        }).then((testGroups) => {
+            assert.equal(testGroups.length, 1);
+            const testGroup = testGroups[0];
+            const webkit = Repository.findById(MockData.webkitRepositoryId());
+            assert.equal(testGroup.buildRequests().length, 6);
+
+            const buildRequest = testGroup.buildRequests()[0];
+            assert(buildRequest.isBuild());
+            assert(!buildRequest.isTest());
+            assert.equal(buildRequest.statusLabel(), 'Completed');
+            assert.equal(buildRequest.statusUrl(), 'http://build.webkit.org/builders/some%20builder/builds/124');
+            assert.notEqual(buildRequest.buildId(), null);
+
+            const commitSet = buildRequest.commitSet();
+            assert.equal(commitSet.revisionForRepository(webkit), '191622');
+            const webkitPatch = commitSet.patchForRepository(webkit);
+            assert(webkitPatch instanceof UploadedFile);
+            assert.equal(webkitPatch.filename(), 'patch.dat');
+            const webkitRoot = commitSet.rootForRepository(webkit);
+            assert(webkitRoot instanceof UploadedFile);
+            assert.equal(webkitRoot.filename(), 'root123.dat');
+            assert.deepEqual(commitSet.allRootFiles(), [webkitRoot]);
+
+            const otherBuildRequest = testGroup.buildRequests()[1];
+            assert(otherBuildRequest.isBuild());
+            assert(!otherBuildRequest.isTest());
+            assert.equal(otherBuildRequest.statusLabel(), 'Waiting');
+            assert.equal(otherBuildRequest.statusUrl(), null);
+            assert.equal(otherBuildRequest.buildId(), null);
+
+            const otherCommitSet = otherBuildRequest.commitSet();
+            assert.equal(otherCommitSet.revisionForRepository(webkit), '191622');
+            assert.equal(otherCommitSet.patchForRepository(webkit), null);
+            assert.equal(otherCommitSet.rootForRepository(webkit), null);
+            assert.deepEqual(otherCommitSet.allRootFiles(), []);
+
+            MockRemoteAPI.reset();
+            syncPromise = triggerable.syncOnce();
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 3);
+            assertAndResolveRequest(requests[0], 'GET', '/json/builders/some%20tester/pendingBuilds', []);
+            assertAndResolveRequest(requests[1], 'GET', '/json/builders/some%20builder/pendingBuilds', []);
+            assertAndResolveRequest(requests[2], 'GET', '/json/builders/other%20builder/pendingBuilds', []);
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 6);
+            assertAndResolveRequest(requests[3], 'GET', '/json/builders/some%20tester/builds/?select=-1&select=-2', {});
+            assertAndResolveRequest(requests[4], 'GET', '/json/builders/some%20builder/builds/?select=-1&select=-2', {
+                [-1]: MockData.finishedBuild({builder: 'some builder', buildRequestId: 1})
+            });
+            assertAndResolveRequest(requests[5], 'GET', '/json/builders/other%20builder/builds/?select=-1&select=-2', {});
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 7);
+            assertAndResolveRequest(requests[6], 'POST', '/builders/some%20builder/force', 'OK');
+            assert.deepEqual(requests[6].data, {'wk': '191622', 'build-request-id': '2', 'forcescheduler': 'force-ab-builds'});
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 10);
+            assertAndResolveRequest(requests[7], 'GET', '/json/builders/some%20tester/pendingBuilds', []);
+            assertAndResolveRequest(requests[8], 'GET', '/json/builders/some%20builder/pendingBuilds', []);
+            assertAndResolveRequest(requests[9], 'GET', '/json/builders/other%20builder/pendingBuilds', []);
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 13);
+            assertAndResolveRequest(requests[10], 'GET', '/json/builders/some%20tester/builds/?select=-1&select=-2', {});
+            assertAndResolveRequest(requests[11], 'GET', '/json/builders/some%20builder/builds/?select=-1&select=-2', {
+                [-1]: MockData.runningBuild({builder: 'some builder', buildRequestId: 2}),
+                [-2]: MockData.finishedBuild({builder: 'some builder', buildRequestId: 1}),
+            });
+            assertAndResolveRequest(requests[12], 'GET', '/json/builders/other%20builder/builds/?select=-1&select=-2', {});
+            return syncPromise;
+        }).then(() => {
+            return TestGroup.fetchForTask(taskId, true);
+        }).then((testGroups) => {
+            assert.equal(testGroups.length, 1);
+            const testGroup = testGroups[0];
+            const webkit = Repository.findById(MockData.webkitRepositoryId());
+            assert.equal(testGroup.buildRequests().length, 6);
+
+            const buildRequest = testGroup.buildRequests()[0];
+            assert(buildRequest.isBuild());
+            assert(!buildRequest.isTest());
+            assert.equal(buildRequest.statusLabel(), 'Completed');
+            assert.equal(buildRequest.statusUrl(), 'http://build.webkit.org/builders/some%20builder/builds/124');
+            assert.notEqual(buildRequest.buildId(), null);
+
+            const commitSet = buildRequest.commitSet();
+            assert.equal(commitSet.revisionForRepository(webkit), '191622');
+            const webkitPatch = commitSet.patchForRepository(webkit);
+            assert(webkitPatch instanceof UploadedFile);
+            assert.equal(webkitPatch.filename(), 'patch.dat');
+            const webkitRoot = commitSet.rootForRepository(webkit);
+            assert(webkitRoot instanceof UploadedFile);
+            assert.equal(webkitRoot.filename(), 'root123.dat');
+            assert.deepEqual(commitSet.allRootFiles(), [webkitRoot]);
+
+            const otherBuildRequest = testGroup.buildRequests()[1];
+            assert(otherBuildRequest.isBuild());
+            assert(!otherBuildRequest.isTest());
+            assert.equal(otherBuildRequest.statusLabel(), 'Running');
+            assert.equal(otherBuildRequest.statusUrl(), 'http://build.webkit.org/builders/some%20builder/builds/124');
+            assert.equal(otherBuildRequest.buildId(), null);
+
+            const otherCommitSet = otherBuildRequest.commitSet();
+            assert.equal(otherCommitSet.revisionForRepository(webkit), '191622');
+            assert.equal(otherCommitSet.patchForRepository(webkit), null);
+            assert.equal(otherCommitSet.rootForRepository(webkit), null);
+            assert.deepEqual(otherCommitSet.allRootFiles(), []);
+
+            return uploadRoot(otherBuildRequest.id(), 124);
+        }).then(() => {
+            return TestGroup.fetchForTask(taskId, true);
+        }).then((testGroups) => {
+            assert.equal(testGroups.length, 1);
+            const testGroup = testGroups[0];
+            const webkit = Repository.findById(MockData.webkitRepositoryId());
+            assert.equal(testGroup.buildRequests().length, 6);
+
+            const buildRequest = testGroup.buildRequests()[0];
+            assert(buildRequest.isBuild());
+            assert(!buildRequest.isTest());
+            assert.equal(buildRequest.statusLabel(), 'Completed');
+            assert.equal(buildRequest.statusUrl(), 'http://build.webkit.org/builders/some%20builder/builds/124');
+            assert.notEqual(buildRequest.buildId(), null);
+
+            const commitSet = buildRequest.commitSet();
+            assert.equal(commitSet.revisionForRepository(webkit), '191622');
+            const webkitPatch = commitSet.patchForRepository(webkit);
+            assert(webkitPatch instanceof UploadedFile);
+            assert.equal(webkitPatch.filename(), 'patch.dat');
+            const webkitRoot = commitSet.rootForRepository(webkit);
+            assert(webkitRoot instanceof UploadedFile);
+            assert.equal(webkitRoot.filename(), 'root123.dat');
+            assert.deepEqual(commitSet.allRootFiles(), [webkitRoot]);
+
+            const otherBuildRequest = testGroup.buildRequests()[1];
+            assert(otherBuildRequest.isBuild());
+            assert(!otherBuildRequest.isTest());
+            assert.equal(otherBuildRequest.statusLabel(), 'Completed');
+            assert.equal(otherBuildRequest.statusUrl(), 'http://build.webkit.org/builders/some%20builder/builds/124');
+            assert.notEqual(otherBuildRequest.buildId(), null);
+
+            const otherCommitSet = otherBuildRequest.commitSet();
+            assert.equal(otherCommitSet.revisionForRepository(webkit), '191622');
+            assert.equal(otherCommitSet.patchForRepository(webkit), null);
+            const otherWebkitRoot = otherCommitSet.rootForRepository(webkit);
+            assert(otherWebkitRoot instanceof UploadedFile);
+            assert.equal(otherWebkitRoot.filename(), 'root124.dat');
+            assert.deepEqual(otherCommitSet.allRootFiles(), [otherWebkitRoot]);
+        });
+    });
+
+    it('should schedule a build to test after building a patch', () => {
+        const requests = MockRemoteAPI.requests;
+        let triggerable;
+        let taskId = null;
+        let syncPromise;
+        let firstRoot = null;
+        return createTriggerable().then((newTriggerable) => {
+            triggerable = newTriggerable;
+            return createTestGroupWihPatch();
+        }).then((testGroup) => {
+            taskId = testGroup.task().id();
+            const webkit = Repository.findById(MockData.webkitRepositoryId());
+            assert.equal(testGroup.buildRequests().length, 6);
+
+            const buildRequest = testGroup.buildRequests()[0];
+            assert.equal(buildRequest.id(), 1);
+            assert(buildRequest.isBuild());
+            assert(!buildRequest.isTest());
+            assert.equal(buildRequest.statusLabel(), 'Waiting');
+            assert.equal(buildRequest.buildId(), null);
+            assert.deepEqual(buildRequest.commitSet().allRootFiles(), []);
+
+            const otherBuildRequest = testGroup.buildRequests()[1];
+            assert.equal(otherBuildRequest.id(), 2);
+            assert(otherBuildRequest.isBuild());
+            assert(!otherBuildRequest.isTest());
+            assert.equal(buildRequest.statusLabel(), 'Waiting');
+            assert.equal(otherBuildRequest.buildId(), null);
+            assert.deepEqual(otherBuildRequest.commitSet().allRootFiles(), []);
+
+            return uploadRoot(1, 45);
+        }).then(() => {
+            return uploadRoot(2, 46);
+        }).then(() => {
+            return TestGroup.fetchForTask(taskId, true);
+        }).then((testGroups) => {
+            assert.equal(testGroups.length, 1);
+            const testGroup = testGroups[0];
+
+            const buildRequest = testGroup.buildRequests()[0];
+            assert(buildRequest.isBuild());
+            assert(!buildRequest.isTest());
+            assert.equal(buildRequest.statusLabel(), 'Completed');
+            assert.notEqual(buildRequest.buildId(), null);
+            const roots = buildRequest.commitSet().allRootFiles();
+            assert.equal(roots.length, 1);
+            firstRoot = roots[0];
+            assert.deepEqual(roots[0].filename(), 'root45.dat');
+
+            const otherBuildRequest = testGroup.buildRequests()[1];
+            assert(otherBuildRequest.isBuild());
+            assert(!otherBuildRequest.isTest());
+            assert.equal(otherBuildRequest.statusLabel(), 'Completed');
+            assert.notEqual(otherBuildRequest.buildId(), null);
+            const otherRoots = otherBuildRequest.commitSet().allRootFiles();
+            assert.equal(otherRoots.length, 1);
+            assert.deepEqual(otherRoots[0].filename(), 'root46.dat');
+            syncPromise = triggerable.syncOnce();
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 3);
+            assertAndResolveRequest(requests[0], 'GET', '/json/builders/some%20tester/pendingBuilds', []);
+            assertAndResolveRequest(requests[1], 'GET', '/json/builders/some%20builder/pendingBuilds', []);
+            assertAndResolveRequest(requests[2], 'GET', '/json/builders/other%20builder/pendingBuilds', []);
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 6);
+            assertAndResolveRequest(requests[3], 'GET', '/json/builders/some%20tester/builds/?select=-1&select=-2', {});
+            assertAndResolveRequest(requests[4], 'GET', '/json/builders/some%20builder/builds/?select=-1&select=-2', {
+                [-1]: MockData.finishedBuild({builder: 'some builder', buildRequestId: 1}),
+                [-2]: MockData.finishedBuild({builder: 'some builder', buildRequestId: 2}),
+            });
+            assertAndResolveRequest(requests[5], 'GET', '/json/builders/other%20builder/builds/?select=-1&select=-2', {});
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 7);
+            assertAndResolveRequest(requests[6], 'POST', '/builders/some%20tester/force', 'OK');
+            assert.deepEqual(requests[6].data, {'test': 'some-test', 'wk': '191622', 'build-request-id': '3', 'forcescheduler': 'force-ab-tests',
+                'roots': JSON.stringify([{url: firstRoot.url()}])});
+            return MockRemoteAPI.waitForRequest();
+        });
+    });
+
+    it('should not schedule a build to test while building a patch', () => {
+        const requests = MockRemoteAPI.requests;
+        let triggerable;
+        let taskId = null;
+        let syncPromise;
+        return createTriggerable().then((newTriggerable) => {
+            triggerable = newTriggerable;
+            return createTestGroupWihPatch();
+        }).then((testGroup) => {
+            taskId = testGroup.task().id();
+            assert.equal(testGroup.buildRequests().length, 6);
+
+            const buildRequest = testGroup.buildRequests()[0];
+            assert.equal(buildRequest.id(), 1);
+            assert(buildRequest.isBuild());
+            assert(!buildRequest.isTest());
+            assert.equal(buildRequest.statusLabel(), 'Waiting');
+            assert.equal(buildRequest.statusUrl(), null);
+            assert.equal(buildRequest.buildId(), null);
+
+            const otherBuildRequest = testGroup.buildRequests()[1];
+            assert.equal(otherBuildRequest.id(), 2);
+            assert(otherBuildRequest.isBuild());
+            assert(!otherBuildRequest.isTest());
+            assert.equal(otherBuildRequest.statusLabel(), 'Waiting');
+            assert.equal(otherBuildRequest.statusUrl(), null);
+            assert.equal(otherBuildRequest.buildId(), null);
+
+            syncPromise = triggerable.syncOnce();
+            return Promise.all([MockRemoteAPI.waitForRequest(), uploadRoot(1, 123)]);
+        }).then(() => {
+            assert.equal(requests.length, 3);
+            assertAndResolveRequest(requests[0], 'GET', '/json/builders/some%20tester/pendingBuilds', []);
+            assertAndResolveRequest(requests[1], 'GET', '/json/builders/some%20builder/pendingBuilds', []);
+            assertAndResolveRequest(requests[2], 'GET', '/json/builders/other%20builder/pendingBuilds', []);
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 6);
+            assertAndResolveRequest(requests[3], 'GET', '/json/builders/some%20tester/builds/?select=-1&select=-2', {});
+            assertAndResolveRequest(requests[4], 'GET', '/json/builders/some%20builder/builds/?select=-1&select=-2', {
+                [-1]: MockData.runningBuild({builder: 'some builder', buildRequestId: 2}),
+                [-2]: MockData.finishedBuild({builder: 'some builder', buildRequestId: 1}),
+            });
+            assertAndResolveRequest(requests[5], 'GET', '/json/builders/other%20builder/builds/?select=-1&select=-2', {});
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 9);
+            assertAndResolveRequest(requests[6], 'GET', '/json/builders/some%20tester/pendingBuilds', []);
+            assertAndResolveRequest(requests[7], 'GET', '/json/builders/some%20builder/pendingBuilds', []);
+            assertAndResolveRequest(requests[8], 'GET', '/json/builders/other%20builder/pendingBuilds', []);
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 12);
+            assertAndResolveRequest(requests[9], 'GET', '/json/builders/some%20tester/builds/?select=-1&select=-2', {});
+            assertAndResolveRequest(requests[10], 'GET', '/json/builders/some%20builder/builds/?select=-1&select=-2', {
+                [-1]: MockData.runningBuild({builder: 'some builder', buildRequestId: 2, buildNumber: 1002}),
+                [-2]: MockData.finishedBuild({builder: 'some builder', buildRequestId: 1}),
+            });
+            assertAndResolveRequest(requests[11], 'GET', '/json/builders/other%20builder/builds/?select=-1&select=-2', {});
+            return syncPromise;
+        }).then(() => {
+            return TestGroup.fetchForTask(taskId, true);
+        }).then((testGroups) => {
+            assert.equal(testGroups.length, 1);
+
+            const testGroup = testGroups[0];
+            const buildRequest = testGroup.buildRequests()[0];
+            assert.equal(buildRequest.id(), 1);
+            assert(buildRequest.isBuild());
+            assert(!buildRequest.isTest());
+            assert.equal(buildRequest.statusLabel(), 'Completed');
+            assert.equal(buildRequest.statusUrl(), null);
+            assert.notEqual(buildRequest.buildId(), null);
+
+            const otherBuildRequest = testGroup.buildRequests()[1];
+            assert.equal(otherBuildRequest.id(), 2);
+            assert(otherBuildRequest.isBuild());
+            assert(!otherBuildRequest.isTest());
+            assert.equal(otherBuildRequest.statusLabel(), 'Running');
+            assert.equal(otherBuildRequest.statusUrl(), 'http://build.webkit.org/builders/some%20builder/builds/1002');
+            assert.equal(otherBuildRequest.buildId(), null);
+        });
+    });
+
+    it('should cancel builds for testing when a build to build a patch fails', () => {
+        const requests = MockRemoteAPI.requests;
+        let triggerable;
+        let taskId = null;
+        let syncPromise;
+        return createTriggerable().then((newTriggerable) => {
+            triggerable = newTriggerable;
+            return createTestGroupWihPatch();
+        }).then((testGroup) => {
+            taskId = testGroup.task().id();
+            assert.equal(testGroup.buildRequests().length, 6);
+
+            const buildRequest = testGroup.buildRequests()[0];
+            assert.equal(buildRequest.id(), 1);
+            assert(buildRequest.isBuild());
+            assert(!buildRequest.isTest());
+            assert.equal(buildRequest.statusLabel(), 'Waiting');
+            assert.equal(buildRequest.statusUrl(), null);
+            assert.equal(buildRequest.buildId(), null);
+
+            const otherBuildRequest = testGroup.buildRequests()[1];
+            assert.equal(otherBuildRequest.id(), 2);
+            assert(otherBuildRequest.isBuild());
+            assert(!otherBuildRequest.isTest());
+            assert.equal(otherBuildRequest.statusLabel(), 'Waiting');
+            assert.equal(otherBuildRequest.statusUrl(), null);
+            assert.equal(otherBuildRequest.buildId(), null);
+
+            syncPromise = triggerable.syncOnce();
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 3);
+            assertAndResolveRequest(requests[0], 'GET', '/json/builders/some%20tester/pendingBuilds', []);
+            assertAndResolveRequest(requests[1], 'GET', '/json/builders/some%20builder/pendingBuilds', []);
+            assertAndResolveRequest(requests[2], 'GET', '/json/builders/other%20builder/pendingBuilds', []);
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 6);
+            assertAndResolveRequest(requests[3], 'GET', '/json/builders/some%20tester/builds/?select=-1&select=-2', {});
+            assertAndResolveRequest(requests[4], 'GET', '/json/builders/some%20builder/builds/?select=-1&select=-2', {});
+            assertAndResolveRequest(requests[5], 'GET', '/json/builders/other%20builder/builds/?select=-1&select=-2', {
+                [-1]: MockData.finishedBuild({builder: 'other builder', buildRequestId: 1, buildNumber: 312}),
+            });
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 9);
+            assertAndResolveRequest(requests[6], 'GET', '/json/builders/some%20tester/pendingBuilds', []);
+            assertAndResolveRequest(requests[7], 'GET', '/json/builders/some%20builder/pendingBuilds', []);
+            assertAndResolveRequest(requests[8], 'GET', '/json/builders/other%20builder/pendingBuilds', []);
+            return MockRemoteAPI.waitForRequest();
+        }).then(() => {
+            assert.equal(requests.length, 12);
+            assertAndResolveRequest(requests[9], 'GET', '/json/builders/some%20tester/builds/?select=-1&select=-2', {});
+            assertAndResolveRequest(requests[10], 'GET', '/json/builders/some%20builder/builds/?select=-1&select=-2', {});
+            assertAndResolveRequest(requests[11], 'GET', '/json/builders/other%20builder/builds/?select=-1&select=-2', {
+                [-1]: MockData.finishedBuild({builder: 'other builder', buildRequestId: 1, buildNumber: 312}),
+            });
+            return syncPromise;
+        }).then(() => {
+            return TestGroup.fetchForTask(taskId, true);
+        }).then((testGroups) => {
+            assert.equal(testGroups.length, 1);
+
+            const buildReqeusts = testGroups[0].buildRequests();
+            assert(buildReqeusts[0].isBuild());
+            assert(!buildReqeusts[0].isTest());
+            assert.equal(buildReqeusts[0].statusLabel(), 'Failed');
+            assert.equal(buildReqeusts[0].statusUrl(), 'http://build.webkit.org/builders/other%20builder/builds/312');
+            assert.equal(buildReqeusts[0].buildId(), null);
+
+            assert(buildReqeusts[1].isBuild());
+            assert(!buildReqeusts[1].isTest());
+            assert.equal(buildReqeusts[1].statusLabel(), 'Failed');
+            assert.equal(buildReqeusts[1].statusUrl(), null);
+            assert.equal(buildReqeusts[1].buildId(), null);
+
+            function assertTestBuildHasFailed(request)
+            {
+                assert(!request.isBuild());
+                assert(request.isTest());
+                assert.equal(request.statusLabel(), 'Failed');
+                assert.equal(request.statusUrl(), null);
+                assert.equal(request.buildId(), null);
+            }
+
+            assertTestBuildHasFailed(buildReqeusts[2]);
+            assertTestBuildHasFailed(buildReqeusts[3]);
+        });
+    });
+
+});
index ee611f3..7abb0c9 100644 (file)
@@ -60,7 +60,8 @@ class BuildbotSyncer {
     constructor(remote, object, commonConfigurations)
     {
         this._remote = remote;
-        this._testConfigurations = [];
+        this._type = null;
+        this._configurations = [];
         this._repositoryGroups = commonConfigurations.repositoryGroups;
         this._slavePropertyName = commonConfigurations.slaveArgument;
         this._buildRequestPropertyName = commonConfigurations.buildRequestArgument;
@@ -76,14 +77,28 @@ class BuildbotSyncer {
     {
         assert(test instanceof Test);
         assert(platform instanceof Platform);
-        this._testConfigurations.push({test, platform, propertiesTemplate});
+        assert(this._type == null || this._type == 'tester');
+        this._type = 'tester';
+        this._configurations.push({test, platform, propertiesTemplate});
     }
-    testConfigurations() { return this._testConfigurations; }
+    testConfigurations() { return this._type == 'tester' ? this._configurations : []; }
+
+    addBuildConfiguration(platform, propertiesTemplate)
+    {
+        assert(platform instanceof Platform);
+        assert(this._type == null || this._type == 'builder');
+        this._type = 'builder';
+        this._configurations.push({test: null, platform, propertiesTemplate});
+    }
+    buildConfigurations() { return this._type == 'builder' ? this._configurations : []; }
+
+    isTester() { return this._type == 'tester'; }
+
     repositoryGroups() { return this._repositoryGroups; }
 
     matchesConfiguration(request)
     {
-        return this._testConfigurations.some((config) => config.platform == request.platform() && config.test == request.test());
+        return this._configurations.some((config) => config.platform == request.platform() && config.test == request.test());
     }
 
     scheduleRequest(newRequest, slaveName)
@@ -202,7 +217,7 @@ class BuildbotSyncer {
         for (let repository of commitSet.repositories())
             repositoryByName[repository.name()] = repository;
 
-        const matchingConfiguration = this._testConfigurations.find((config) => config.platform == buildRequest.platform() && config.test == buildRequest.test());
+        const matchingConfiguration = this._configurations.find((config) => config.platform == buildRequest.platform() && config.test == buildRequest.test());
         assert(matchingConfiguration, `Build request ${buildRequest.id()} does not match a configuration in the builder "${this._builderName}"`);
         const propertiesTemplate = matchingConfiguration.propertiesTemplate;
 
@@ -216,10 +231,30 @@ class BuildbotSyncer {
         for (let propertyName in propertiesTemplate)
             properties[propertyName] = propertiesTemplate[propertyName];
 
-        const repositoryGroupTemplate = repositoryGroupConfiguration.propertiesTemplate;
+        const repositoryGroupTemplate = buildRequest.isBuild() ? repositoryGroupConfiguration.buildPropertiesTemplate : repositoryGroupConfiguration.testPropertiesTemplate;
         for (let propertyName in repositoryGroupTemplate) {
-            const value = repositoryGroupTemplate[propertyName];
-            properties[propertyName] = value instanceof Repository ? commitSet.revisionForRepository(value) : value;
+            let value = repositoryGroupTemplate[propertyName];
+            const type = typeof(value) == 'object' ? value.type : 'string';
+            switch (type) {
+            case 'string':
+                break;
+            case 'revision':
+                value = commitSet.revisionForRepository(value.repository);
+                break;
+            case 'patch':
+                const patch = commitSet.patchForRepository(value.repository);
+                if (!patch)
+                    continue;
+                value = patch.url();
+                break;
+            case 'roots':
+                const rootFiles = commitSet.allRootFiles();
+                if (!rootFiles.length)
+                    continue;
+                value = JSON.stringify(rootFiles.map((file) => ({url: file.url()})));
+                break;
+            }
+            properties[propertyName] = value;
         }
         properties[this._buildRequestPropertyName] = buildRequest.id();
 
@@ -262,121 +297,183 @@ class BuildbotSyncer {
             buildRequestArgument: config.buildRequestArgument,
         };
 
-        let syncerByBuilder = new Map;
-        const expandedConfigurations = [];
-        for (let entry of config['configurations']) {
-            for (const expandedConfig of this._expandTypesAndPlatforms(entry))
-                expandedConfigurations.push(expandedConfig);
+        const syncerByBuilder = new Map;
+        const ensureBuildbotSyncer = (builderInfo) => {
+            let builderSyncer = syncerByBuilder.get(builderInfo.builder);
+            if (!builderSyncer) {
+                builderSyncer = new BuildbotSyncer(remote, builderInfo, commonConfigurations);
+                syncerByBuilder.set(builderInfo.builder, builderSyncer);
+            }
+            return builderSyncer;
         }
 
-        for (let entry of expandedConfigurations) {
-            const mergedConfig = {};
-            this._validateAndMergeConfig(mergedConfig, entry);
+        assert(Array.isArray(config['testConfigurations']), `The test configuration must be an array`);
+        this._resolveBuildersWithPlatforms('test', config['testConfigurations'], builders).forEach((entry, configurationIndex) => {
+            assert(Array.isArray(entry['types']), `The test configuration ${configurationIndex} does not specify "types" as an array`);
+            for (const type of entry['types']) {
+                const typeConfig = this._validateAndMergeConfig({}, entry.builderConfig);
+                assert(types[type], `"${type}" is not a valid type in the configuration`);
+                this._validateAndMergeConfig(typeConfig, types[type]);
 
-            if ('type' in mergedConfig) {
-                const type = mergedConfig['type'];
-                assert(type, `${type} is not a valid type in the configuration`);
-                this._validateAndMergeConfig(mergedConfig, types[type]);
-            }
+                const testPath = typeConfig.test;
+                const test = Test.findByPath(testPath);
+                assert(test, `"${testPath.join('", "')}" is not a valid test path in the test configuration ${configurationIndex}`);
 
-            const builder = entry['builder'];
-            if (builders[builder])
-                this._validateAndMergeConfig(mergedConfig, builders[builder]);
+                ensureBuildbotSyncer(entry.builderConfig).addTestConfiguration(test, entry.platform, typeConfig.properties);
+            }
+        });
 
-            this._createTestConfiguration(remote, syncerByBuilder, mergedConfig, commonConfigurations);
+        const buildConfigurations = config['buildConfigurations'];
+        if (buildConfigurations) {
+            assert(Array.isArray(buildConfigurations), `The test configuration must be an array`);
+            this._resolveBuildersWithPlatforms('test', buildConfigurations, builders).forEach((entry, configurationIndex) => {
+                const syncer = ensureBuildbotSyncer(entry.builderConfig);
+                assert(!syncer.isTester(), `The build configuration ${configurationIndex} uses a tester: ${syncer.builderName()}`);
+                syncer.addBuildConfiguration(entry.platform, entry.builderConfig.properties);
+            });
         }
 
         return Array.from(syncerByBuilder.values());
     }
 
+    static _resolveBuildersWithPlatforms(configurationType, configurationList, builders)
+    {
+        const resolvedConfigurations = [];
+        let configurationIndex = 0;
+        for (const entry of configurationList) {
+            configurationIndex++;
+            assert(Array.isArray(entry['builders']), `The ${configurationType} configuration ${configurationIndex} does not specify "builders" as an array`);
+            assert(Array.isArray(entry['platforms']), `The ${configurationType} configuration ${configurationIndex} does not specify "platforms" as an array`);
+            for (const builderKey of entry['builders']) {
+                const matchingBuilder = builders[builderKey];
+                assert(matchingBuilder, `"${builderKey}" is not a valid builder in the configuration`);
+                assert('builder' in matchingBuilder, `Builder ${builderKey} does not specify a buildbot builder name`);
+                const builderConfig = this._validateAndMergeConfig({}, matchingBuilder);
+                for (const platformName of entry['platforms']) {
+                    const platform = Platform.findByName(platformName);
+                    assert(platform, `${platformName} is not a valid platform name`);
+                    resolvedConfigurations.push({types: entry.types, builderConfig, platform});
+                }
+            }
+        }
+        return resolvedConfigurations;
+    }
+
     static _parseRepositoryGroup(name, group)
     {
         assert.equal(typeof(group.repositories), 'object',
             `Repository group "${name}" does not specify a dictionary of repositories`);
         assert(!('description' in group) || typeof(group['description']) == 'string',
             `Repository group "${name}" have an invalid description`);
-        assert.equal(typeof(group.properties), 'object', `Repository group "${name}" specifies an invalid dictionary of properties`);
         assert([undefined, true, false].includes(group.acceptsRoots),
-            `Repository group "${name}" contains invalid acceptsRoots value: ${group.acceptsRoots}`);
+            `Repository group "${name}" contains invalid acceptsRoots value: ${JSON.stringify(group.acceptsRoots)}`);
 
         const repositoryByName = {};
         const parsedRepositoryList = [];
+        const patchAcceptingRepositoryList = new Set;
         for (const repositoryName in group.repositories) {
             const options = group.repositories[repositoryName];
             const repository = Repository.findTopLevelByName(repositoryName);
             assert(repository, `"${repositoryName}" is not a valid repository name`);
             repositoryByName[repositoryName] = repository;
-            assert.equal(typeof(options), 'object', `"${repositoryName}" does not specify a valid option`);
+            assert.equal(typeof(options), 'object', `"${repositoryName}" specifies a non-dictionary value`);
             assert([undefined, true, false].includes(options.acceptsPatch),
-                `"${repositoryName}" contains invalid acceptsPatch value: ${options.acceptsPatch}`);
+                `"${repositoryName}" contains invalid acceptsPatch value: ${JSON.stringify(options.acceptsPatch)}`);
+            if (options.acceptsPatch)
+                patchAcceptingRepositoryList.add(repository);
             repositoryByName[repositoryName] = repository;
             parsedRepositoryList.push({repository: repository.id(), acceptsPatch: options.acceptsPatch});
         }
+        assert(parsedRepositoryList.length, `Repository group "${name}" does not specify any repository`);
 
-        const propertiesTemplate = {};
-        const usedRepositories = [];
-        for (const propertyName in group.properties) {
-            let value = group.properties[propertyName];
-            const match = value.match(/^<(.+)>$/);
-            if (match) {
-                const repositoryName = match[1];
-                value = repositoryByName[repositoryName];
-                assert(value, `Repository group "${name}" uses "${repositoryName}" in its property but does not list in the list of repositories`);
-                usedRepositories.push(value);
+        assert.equal(typeof(group.testProperties), 'object', `Repository group "${name}" specifies the test configurations with an invalid type`);
+
+        const resolveRepository = (repositoryName) => {
+            const repository = repositoryByName[repositoryName];
+            assert(repository, `Repository group "${name}" an invalid repository "${repositoryName}"`);
+            return repository;
+        }
+
+        const testRepositories = new Set;
+        let specifiesRoots = false;
+        const testPropertiesTemplate = this._parseRepositoryGroupPropertyTemplate('test', name, group.testProperties, (type, value) => {
+            assert(type != 'patch', `Repository group "${name}" specifies a patch for "${value}" in the properties for testing`);
+            switch (type) {
+            case 'revision':
+                const repository = resolveRepository(value);
+                testRepositories.add(repository);
+                return {type, repository};
+            case 'roots':
+                assert(group.acceptsRoots, `Repository group "${name}" specifies roots in a property but it does not accept roots`);
+                specifiesRoots = true;
+                return {type};
             }
-            propertiesTemplate[propertyName] = value;
+            return null;
+        });
+        assert(!group.acceptsRoots == !specifiesRoots,
+            `Repository group "${name}" accepts roots but does not specify roots in testProperties`);
+        assert.equal(parsedRepositoryList.length, testRepositories.size,
+            `Repository group "${name}" does not use some of the repositories listed in testing`);
+
+        let buildPropertiesTemplate = null;
+        if ('buildProperties' in group) {
+            assert(patchAcceptingRepositoryList.size, `Repository group "${name}" specifies the properties for building but does not accept any patches`);
+            assert(group.acceptsRoots, `Repository group "${name}" specifies the properties for building but does not accept roots in testing`);
+            const revisionRepositories = new Set;
+            const patchRepositories = new Set;
+            buildPropertiesTemplate = this._parseRepositoryGroupPropertyTemplate('build', name, group.buildProperties, (type, value) => {
+                assert(type != 'roots', `Repository group "${name}" specifies roots in the properties for building`);
+                const repository = resolveRepository(value);
+                switch (type) {
+                case 'patch':
+                    assert(patchAcceptingRepositoryList.has(repository), `Repository group "${name}" specifies a patch for "${value}" but it does not accept a patch`);
+                    patchRepositories.add(repository);
+                    return {type, repository};
+                case 'revision':
+                    revisionRepositories.add(repository);
+                    return {type, repository};
+                }
+                return null;
+            });
+            for (const repository of patchRepositories)
+                assert(revisionRepositories.has(repository), `Repository group "${name}" specifies a patch for "${repository.name()}" but does not specify a revision`);
+            assert.equal(patchAcceptingRepositoryList.size, patchRepositories.size,
+                `Repository group "${name}" does not use some of the repositories listed in building a patch`);
         }
-        assert(parsedRepositoryList.length, `Repository group "${name}" does not specify any repository`);
-        assert.equal(parsedRepositoryList.length, usedRepositories.length,
-            `Repository group "${name}" does not use some of the repositories listed`);
+
         return {
             name: group.name,
             description: group.description,
             acceptsRoots: group.acceptsRoots,
-            propertiesTemplate,
+            testPropertiesTemplate: testPropertiesTemplate,
+            buildPropertiesTemplate: buildPropertiesTemplate,
             repositoryList: parsedRepositoryList,
         };
     }
 
-    static _expandTypesAndPlatforms(unresolvedConfig)
-    {
-        const typeExpanded = [];
-        if ('types' in unresolvedConfig) {
-            for (let type of unresolvedConfig['types'])
-                typeExpanded.push(this._validateAndMergeConfig({'type': type}, unresolvedConfig, 'types'));
-        } else
-            typeExpanded.push(unresolvedConfig);
-
-        const configurations = [];
-        for (let config of typeExpanded) {
-            if ('platforms' in config) {
-                for (let platform of config['platforms'])
-                    configurations.push(this._validateAndMergeConfig({'platform': platform}, config, 'platforms'));
-            } else
-                configurations.push(config);
-        }
-
-        return configurations;
-    }
-
-    static _createTestConfiguration(remote, syncerByBuilder, newConfig, commonConfigurations)
+    static _parseRepositoryGroupPropertyTemplate(parsingMode, groupName, properties, makeOption)
     {
-        assert('platform' in newConfig, 'configuration must specify a platform');
-        assert('test' in newConfig, 'configuration must specify a test');
-        assert('builder' in newConfig, 'configuration must specify a builder');
-
-        const test = Test.findByPath(newConfig.test);
-        assert(test, `${newConfig.test} is not a valid test path`);
-
-        const platform = Platform.findByName(newConfig.platform);
-        assert(platform, `${newConfig.platform} is not a valid platform name`);
+        const propertiesTemplate = {};
+        for (const propertyName in properties) {
+            let value = properties[propertyName];
+            const isDictionary = typeof(value) == 'object';
+            assert(isDictionary || typeof(value) == 'string',
+                `Repository group "${groupName}" uses an invalid value "${value}" in property "${propertyName}"`);
+
+            if (!isDictionary) {
+                propertiesTemplate[propertyName] = value;
+                continue;
+            }
 
-        let syncer = syncerByBuilder.get(newConfig.builder);
-        if (!syncer) {
-            syncer = new BuildbotSyncer(remote, newConfig, commonConfigurations);
-            syncerByBuilder.set(newConfig.builder, syncer);
+            const keys = Object.keys(value);
+            assert.equal(keys.length, 1,
+                `Repository group "${groupName}" specifies more than one type in property "${propertyName}": "${keys.join('", "')}"`);
+            const type = keys[0];
+            const option = makeOption(type, value[type]);
+            assert(option, `Repository group "${groupName}" specifies an invalid type "${type}" in property "${propertyName}"`);
+            propertiesTemplate[propertyName] = option;
         }
-        syncer.addTestConfiguration(test, platform, newConfig.properties || {});
+        return propertiesTemplate;
     }
 
     static _validateAndMergeConfig(config, valuesToMerge, excludedProperty)
@@ -388,31 +485,27 @@ class BuildbotSyncer {
 
             switch (name) {
             case 'properties': // Fallthrough
-                assert.equal(typeof(value), 'object', 'properties should be a dictionary');
+                assert.equal(typeof(value), 'object', 'Build properties should be a dictionary');
                 if (!config['properties'])
                     config['properties'] = {};
                 const properties = config['properties'];
                 for (const name in value) {
-                    assert.equal(typeof(value[name]), 'string', 'A argument value must be a string');
+                    assert.equal(typeof(value[name]), 'string', `Build properties "${name}" specifies a non-string value of type "${typeof(value)}"`);
                     properties[name] = value[name];
                 }
                 break;
             case 'test': // Fallthrough
             case 'slaveList': // Fallthrough
-            case 'platforms': // Fallthrough
-            case 'types':
                 assert(value instanceof Array, `${name} should be an array`);
                 assert(value.every(function (part) { return typeof part == 'string'; }), `${name} should be an array of strings`);
                 config[name] = value.slice();
                 break;
-            case 'type': // Fallthrough
             case 'builder': // Fallthrough
-            case 'platform': // Fallthrough
                 assert.equal(typeof(value), 'string', `${name} should be of string type`);
                 config[name] = value;
                 break;
             default:
-                assert(false, `Unrecognized parameter ${name}`);
+                assert(false, `Unrecognized parameter "${name}"`);
             }
         }
         return config;
index 2415f70..2150fc6 100644 (file)
@@ -74,7 +74,7 @@ class BuildbotTriggerable {
                 const nextRequest = this._nextRequestInGroup(group, updates);
                 if (!validRequests.has(nextRequest))
                     continue;
-                const promise = this._scheduleRequestIfSlaveIsAvailable(nextRequest, group.syncer, group.slaveName);
+                const promise = this._scheduleRequestIfSlaveIsAvailable(nextRequest, group.requests, group.syncer, group.slaveName);
                 if (promise)
                     promistList.push(promise);
             }
@@ -97,9 +97,10 @@ class BuildbotTriggerable {
         const validatedRequests = new Set;
         for (let request of buildRequests) {
             if (!this._syncers.some((syncer) => syncer.matchesConfiguration(request))) {
-                const key = request.platform().id + '-' + request.test().id();
+                const key = request.platform().id + '-' + (request.isBuild() ? 'build' : request.test().id());
+                const kind = request.isBuild() ? 'Building' : `"${request.test().fullName()}"`;
                 if (!(key in testPlatformPairs))
-                    this._logger.error(`Build request ${request.id()} has no matching configuration: "${request.test().fullName()}" on "${request.platform().name()}".`);
+                    this._logger.error(`Build request ${request.id()} has no matching configuration: ${kind} on "${request.platform().name()}".`);
                 testPlatformPairs[key] = true;
                 continue;
             }
@@ -175,21 +176,25 @@ class BuildbotTriggerable {
                 return null;
             if (request.isPending() && !(request.id() in pendingUpdates))
                 return request;
+            if (request.isBuild() && !request.hasCompleted())
+                return null; // A build request is still pending, scheduled, running, or failed.
         }
         return null;
     }
 
-    _scheduleRequestIfSlaveIsAvailable(nextRequest, syncer, slaveName)
+    _scheduleRequestIfSlaveIsAvailable(nextRequest, requestsInGroup, syncer, slaveName)
     {
         if (!nextRequest)
             return null;
 
-        if (!!nextRequest.order()) {
+        const isFirstRequest = nextRequest == requestsInGroup[0] || !nextRequest.order();
+        if (!isFirstRequest) {
             if (syncer)
                 return this._scheduleRequestWithLog(syncer, nextRequest, slaveName);
             this._logger.error(`Could not identify the syncer for ${nextRequest.id()}.`);
         }
 
+        // Pick a new syncer for the first test.
         for (const syncer of this._syncers) {
             const promise = this._scheduleRequestWithLog(syncer, nextRequest, null);
             if (promise)
index 2e1520a..dd272ff 100755 (executable)
@@ -21,20 +21,6 @@ function main(argv)
 
 function syncLoop(options)
 {
-    // FIXME: Fix Manifest.fetch() to use ensureSingleton to create model objects.
-    global.AnalysisTask._fetchAllPromise = null;
-    global.AnalysisTask.clearStaticMap();
-    global.BuildRequest.clearStaticMap();
-    global.CommitLog.clearStaticMap();
-    global.Metric.clearStaticMap();
-    global.Platform.clearStaticMap();
-    global.Repository.clearStaticMap();
-    global.CommitSet.clearStaticMap();
-    global.Test.clearStaticMap();
-    global.TestGroup.clearStaticMap();
-    global.Triggerable.clearStaticMap();
-    global.TriggerableRepositoryGroup.clearStaticMap();
-
     let serverConfig = JSON.parse(fs.readFileSync(options['--server-config-json'], 'utf8'));
     let buildbotConfig = JSON.parse(fs.readFileSync(options['--buildbot-config-json'], 'utf8'));
     let buildbotRemote = new RemoteAPI(buildbotConfig.server);
@@ -44,19 +30,23 @@ function syncLoop(options)
 
     console.log(`Fetching the manifest...`);
 
-    let triggerable;
-    Manifest.fetch().then(function () {
-        triggerable = new BuildbotTriggerable(buildbotConfig, global.RemoteAPI, buildbotRemote, serverConfig.slave, console);
-        return triggerable.updateTriggerable();
-    }).then(function () {
+    const makeTriggerable = function () {
+        return new BuildbotTriggerable(buildbotConfig, global.RemoteAPI, buildbotRemote, serverConfig.slave, console)
+    }
+
+    Manifest.fetch().then(() => {
+        return makeTriggerable().updateTriggerable();
+    }).then(() => {
+        return Manifest.fetch();
+    }).then(() => {
         return triggerable.syncOnce();
-    }).catch(function (error) {
+    }).catch((error) => {
         console.error(error);
         if (typeof(error.stack) == 'string') {
             for (let line of error.stack.split('\n'))
                 console.error(line);
         }
-    }).then(function () {
+    }).then(() => {
         setTimeout(syncLoop.bind(global, options), options['--seconds-to-sleep'] * 1000);
     });
 }
index a4aba78..1c437a9 100644 (file)
@@ -17,9 +17,9 @@ function sampleiOSConfig()
         'repositoryGroups': {
             'ios-svn-webkit': {
                 'repositories': {'WebKit': {}, 'iOS': {}},
-                'properties': {
-                    'desired_image': '<iOS>',
-                    'opensource': '<WebKit>',
+                'testProperties': {
+                    'desired_image': {'revision': 'iOS'},
+                    'opensource': {'revision': 'WebKit'},
                 }
             }
         },
@@ -40,22 +40,25 @@ function sampleiOSConfig()
         'builders': {
             'iPhone-bench': {
                 'builder': 'ABTest-iPhone-RunBenchmark-Tests',
-                'properties': { 'forcescheduler': 'ABTest-iPhone-RunBenchmark-Tests-ForceScheduler' },
+                'properties': {'forcescheduler': 'ABTest-iPhone-RunBenchmark-Tests-ForceScheduler'},
                 'slaveList': ['ABTest-iPhone-0'],
             },
             'iPad-bench': {
                 'builder': 'ABTest-iPad-RunBenchmark-Tests',
-                'properties': { 'forcescheduler': 'ABTest-iPad-RunBenchmark-Tests-ForceScheduler' },
+                'properties': {'forcescheduler': 'ABTest-iPad-RunBenchmark-Tests-ForceScheduler'},
                 'slaveList': ['ABTest-iPad-0', 'ABTest-iPad-1'],
-            }
+            },
+            'iOS-builder': {
+                'builder': 'ABTest-iOS-Builder',
+                'properties': {'forcescheduler': 'ABTest-Builder-ForceScheduler'},
+            },
         },
-        'configurations': [
-            {'type': 'speedometer', 'builder': 'iPhone-bench', 'platform': 'iPhone'},
-            {'type': 'jetstream', 'builder': 'iPhone-bench', 'platform': 'iPhone'},
-            {'type': 'dromaeo-dom', 'builder': 'iPhone-bench', 'platform': 'iPhone'},
-
-            {'type': 'speedometer', 'builder': 'iPad-bench', 'platform': 'iPad'},
-            {'type': 'jetstream', 'builder': 'iPad-bench', 'platform': 'iPad'},
+        'buildConfigurations': [
+            {'builders': ['iOS-builder'], 'platforms': ['iPhone', 'iPad']},
+        ],
+        'testConfigurations': [
+            {'builders': ['iPhone-bench'], 'types': ['speedometer', 'jetstream', 'dromaeo-dom'], 'platforms': ['iPhone']},
+            {'builders': ['iPad-bench'], 'types': ['speedometer', 'jetstream'], 'platforms': ['iPad']},
         ]
     };
 }
@@ -83,21 +86,25 @@ function sampleiOSConfigWithExpansions()
         "builders": {
             "iphone": {
                 "builder": "iPhone AB Tests",
-                "properties": {"forcescheduler": "force-iphone-ab-tests"}
+                "properties": {"forcescheduler": "force-iphone-ab-tests"},
+            },
+            "iphone-2": {
+                "builder": "iPhone 2 AB Tests",
+                "properties": {"forcescheduler": "force-iphone-2-ab-tests"},
             },
             "ipad": {
                 "builder": "iPad AB Tests",
-                "properties": {"forcescheduler": "force-ipad-ab-tests"}
+                "properties": {"forcescheduler": "force-ipad-ab-tests"},
             },
         },
-        "configurations": [
+        "testConfigurations": [
             {
-                "builder": "iphone",
+                "builders": ["iphone", "iphone-2"],
                 "platforms": ["iPhone", "iOS 10 iPhone"],
                 "types": ["iphone-plt", "speedometer"],
             },
             {
-                "builder": "ipad",
+                "builders": ["ipad"],
                 "platforms": ["iPad"],
                 "types": ["ipad-plt", "speedometer"],
             },
@@ -112,16 +119,26 @@ function smallConfiguration()
         'repositoryGroups': {
             'ios-svn-webkit': {
                 'repositories': {'iOS': {}, 'WebKit': {}},
-                'properties': {
-                    'os': '<iOS>',
-                    'wk': '<WebKit>'
+                'testProperties': {
+                    'os': {'revision': 'iOS'},
+                    'wk': {'revision': 'WebKit'}
                 }
             }
         },
-        'configurations': [{
-            'builder': 'some builder',
-            'platform': 'Some platform',
-            'test': ['Some test']
+        'types': {
+            'some-test': {
+                'test': ['Some test'],
+            }
+        },
+        'builders': {
+            'some-builder': {
+                'builder': 'some builder',
+            }
+        },
+        'testConfigurations': [{
+            'builders': ['some-builder'],
+            'platforms': ['Some platform'],
+            'types': ['some-test'],
         }]
     };
 }
@@ -407,82 +424,170 @@ describe('BuildbotSyncer', () => {
         it('should throw when some required options are missing', () => {
             assert.throws(() => {
                 const config = smallConfiguration();
-                delete config.configurations[0].builder;
+                delete config.builders;
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /"some-builder" is not a valid builder in the configuration/);
+            assert.throws(() => {
+                const config = smallConfiguration();
+                delete config.types;
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            }, 'builder should be a required option');
+            }, /"some-test" is not a valid type in the configuration/);
             assert.throws(() => {
                 const config = smallConfiguration();
-                delete config.configurations[0].platform;
+                delete config.testConfigurations[0].builders;
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            }, 'platform should be a required option');
+            }, /The test configuration 1 does not specify "builders" as an array/);
             assert.throws(() => {
                 const config = smallConfiguration();
-                delete config.configurations[0].test;
+                delete config.testConfigurations[0].platforms;
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            }, 'test should be a required option');
+            }, /The test configuration 1 does not specify "platforms" as an array/);
+            assert.throws(() => {
+                const config = smallConfiguration();
+                delete config.testConfigurations[0].types;
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /The test configuration 0 does not specify "types" as an array/);
             assert.throws(() => {
                 const config = smallConfiguration();
                 delete config.buildRequestArgument;
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            }, 'buildRequestArgument should be required');
+            }, /buildRequestArgument must specify the name of the property used to store the build request ID/);
         });
 
         it('should throw when a test name is not an array of strings', () => {
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.configurations[0].test = 'some test';
+                config.testConfigurations[0].types = 'some test';
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /The test configuration 0 does not specify "types" as an array/);
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.configurations[0].test = [1];
+                config.testConfigurations[0].types = [1];
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /"1" is not a valid type in the configuration/);
         });
 
         it('should throw when properties is not an object', () => {
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.configurations[0].properties = 'hello';
+                config.builders[Object.keys(config.builders)[0]].properties = 'hello';
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /Build properties should be a dictionary/);
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.types[Object.keys(config.types)[0]].properties = 'hello';
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Build properties should be a dictionary/);
+        });
+
+        it('should throw when testProperties is specifed in a type or a builder', () => {
+            assert.throws(() => {
+                const config = smallConfiguration();
+                const firstType = Object.keys(config.types)[0];
+                config.types[firstType].testProperties = {};
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Unrecognized parameter "testProperties"/);
+            assert.throws(() => {
+                const config = smallConfiguration();
+                const firstBuilder = Object.keys(config.builders)[0];
+                config.builders[firstBuilder].testProperties = {};
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Unrecognized parameter "testProperties"/);
+        });
+
+        it('should throw when buildProperties is specifed in a type or a builder', () => {
+            assert.throws(() => {
+                const config = smallConfiguration();
+                const firstType = Object.keys(config.types)[0];
+                config.types[firstType].buildProperties = {};
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Unrecognized parameter "buildProperties"/);
+            assert.throws(() => {
+                const config = smallConfiguration();
+                const firstBuilder = Object.keys(config.builders)[0];
+                config.builders[firstBuilder].buildProperties = {};
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Unrecognized parameter "buildProperties"/);
         });
 
-        it('should throw when propertie\'s values are malformed', () => {
+        it('should throw when properties for a type is malformed', () => {
+            const firstType = Object.keys(smallConfiguration().types)[0];
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.configurations[0].properties = {'some': {'otherKey': 'some root'}};
+                config.types[firstType].properties = 'hello';
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Build properties should be a dictionary/);
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.types[firstType].properties = {'some': {'otherKey': 'some root'}};
                 BuildbotSyncer._loadConfig(RemoteAPI, config);
-            });
+            }, /Build properties "some" specifies a non-string value of type "object"/);
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.configurations[0].properties = {'some': {'root': ['a', 'b']}};
+                config.types[firstType].properties = {'some': {'otherKey': 'some root'}};
                 BuildbotSyncer._loadConfig(RemoteAPI, config);
-            });
+            }, /Build properties "some" specifies a non-string value of type "object"/);
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.configurations[0].properties = {'some': {'root': 1}};
+                config.types[firstType].properties = {'some': {'revision': 'WebKit'}};
                 BuildbotSyncer._loadConfig(RemoteAPI, config);
-            });
+            }, /Build properties "some" specifies a non-string value of type "object"/);
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.types[firstType].properties = {'some': 1};
+                BuildbotSyncer._loadConfig(RemoteAPI, config);
+            }, / Build properties "some" specifies a non-string value of type "object"/);
+        });
+
+        it('should throw when properties for a builder is malformed', () => {
+            const firstBuilder = Object.keys(smallConfiguration().builders)[0];
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.builders[firstBuilder].properties = 'hello';
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Build properties should be a dictionary/);
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.builders[firstBuilder].properties = {'some': {'otherKey': 'some root'}};
+                BuildbotSyncer._loadConfig(RemoteAPI, config);
+            }, /Build properties "some" specifies a non-string value of type "object"/);
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.builders[firstBuilder].properties = {'some': {'otherKey': 'some root'}};
+                BuildbotSyncer._loadConfig(RemoteAPI, config);
+            }, /Build properties "some" specifies a non-string value of type "object"/);
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.builders[firstBuilder].properties = {'some': {'revision': 'WebKit'}};
+                BuildbotSyncer._loadConfig(RemoteAPI, config);
+            }, /Build properties "some" specifies a non-string value of type "object"/);
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.builders[firstBuilder].properties = {'some': 1};
+                BuildbotSyncer._loadConfig(RemoteAPI, config);
+            }, /Build properties "some" specifies a non-string value of type "object"/);
         });
 
         it('should create BuildbotSyncer objects for valid configurations', () => {
             let syncers = BuildbotSyncer._loadConfig(RemoteAPI, sampleiOSConfig());
-            assert.equal(syncers.length, 2);
+            assert.equal(syncers.length, 3);
             assert.ok(syncers[0] instanceof BuildbotSyncer);
             assert.ok(syncers[1] instanceof BuildbotSyncer);
+            assert.ok(syncers[2] instanceof BuildbotSyncer);
         });
 
         it('should parse builder names correctly', () => {
             let syncers = BuildbotSyncer._loadConfig(RemoteAPI, sampleiOSConfig());
             assert.equal(syncers[0].builderName(), 'ABTest-iPhone-RunBenchmark-Tests');
             assert.equal(syncers[1].builderName(), 'ABTest-iPad-RunBenchmark-Tests');
+            assert.equal(syncers[2].builderName(), 'ABTest-iOS-Builder');
         });
 
-        it('should parse test configurations correctly', () => {
+        it('should parse test configurations with build configurations correctly', () => {
             let syncers = BuildbotSyncer._loadConfig(RemoteAPI, sampleiOSConfig());
 
             let configurations = syncers[0].testConfigurations();
+            assert(syncers[0].isTester());
             assert.equal(configurations.length, 3);
             assert.equal(configurations[0].platform, MockModels.iphone);
             assert.equal(configurations[0].test, MockModels.speedometer);
@@ -490,37 +595,71 @@ describe('BuildbotSyncer', () => {
             assert.equal(configurations[1].test, MockModels.jetstream);
             assert.equal(configurations[2].platform, MockModels.iphone);
             assert.equal(configurations[2].test, MockModels.domcore);
+            assert.deepEqual(syncers[0].buildConfigurations(), []);
 
             configurations = syncers[1].testConfigurations();
+            assert(syncers[1].isTester());
             assert.equal(configurations.length, 2);
             assert.equal(configurations[0].platform, MockModels.ipad);
             assert.equal(configurations[0].test, MockModels.speedometer);
             assert.equal(configurations[1].platform, MockModels.ipad);
             assert.equal(configurations[1].test, MockModels.jetstream);
+            assert.deepEqual(syncers[1].buildConfigurations(), []);
+
+            assert(!syncers[2].isTester());
+            assert.deepEqual(syncers[2].testConfigurations(), []);
+            configurations = syncers[2].buildConfigurations();
+            assert.equal(configurations.length, 2);
+            assert.equal(configurations[0].platform, MockModels.iphone);
+            assert.equal(configurations[0].test, null);
+            assert.equal(configurations[1].platform, MockModels.ipad);
+            assert.equal(configurations[1].test, null);
+        });
+
+        it('should throw when a build configuration use the same builder as a test configuration', () => {
+            assert.throws(() => {
+                const config = sampleiOSConfig();
+                config.buildConfigurations[0].builders = config.testConfigurations[0].builders;
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            });
         });
 
         it('should parse test configurations with types and platforms expansions correctly', () => {
-            let syncers = BuildbotSyncer._loadConfig(RemoteAPI, sampleiOSConfigWithExpansions());
+            const syncers = BuildbotSyncer._loadConfig(RemoteAPI, sampleiOSConfigWithExpansions());
 
-            assert.equal(syncers.length, 2);
+            assert.equal(syncers.length, 3);
 
             let configurations = syncers[0].testConfigurations();
             assert.equal(configurations.length, 4);
             assert.equal(configurations[0].platform, MockModels.iphone);
             assert.equal(configurations[0].test, MockModels.iPhonePLT);
-            assert.equal(configurations[1].platform, MockModels.iOS10iPhone);
-            assert.equal(configurations[1].test, MockModels.iPhonePLT);
-            assert.equal(configurations[2].platform, MockModels.iphone);
-            assert.equal(configurations[2].test, MockModels.speedometer);
+            assert.equal(configurations[1].platform, MockModels.iphone);
+            assert.equal(configurations[1].test, MockModels.speedometer);
+            assert.equal(configurations[2].platform, MockModels.iOS10iPhone);
+            assert.equal(configurations[2].test, MockModels.iPhonePLT);
             assert.equal(configurations[3].platform, MockModels.iOS10iPhone);
             assert.equal(configurations[3].test, MockModels.speedometer);
+            assert.deepEqual(syncers[0].buildConfigurations(), []);
 
             configurations = syncers[1].testConfigurations();
+            assert.equal(configurations.length, 4);
+            assert.equal(configurations[0].platform, MockModels.iphone);
+            assert.equal(configurations[0].test, MockModels.iPhonePLT);
+            assert.equal(configurations[1].platform, MockModels.iphone);
+            assert.equal(configurations[1].test, MockModels.speedometer);
+            assert.equal(configurations[2].platform, MockModels.iOS10iPhone);
+            assert.equal(configurations[2].test, MockModels.iPhonePLT);
+            assert.equal(configurations[3].platform, MockModels.iOS10iPhone);
+            assert.equal(configurations[3].test, MockModels.speedometer);
+            assert.deepEqual(syncers[1].buildConfigurations(), []);
+
+            configurations = syncers[2].testConfigurations();
             assert.equal(configurations.length, 2);
             assert.equal(configurations[0].platform, MockModels.ipad);
             assert.equal(configurations[0].test, MockModels.iPadPLT);
             assert.equal(configurations[1].platform, MockModels.ipad);
             assert.equal(configurations[1].test, MockModels.speedometer);
+            assert.deepEqual(syncers[2].buildConfigurations(), []);
         });
 
         it('should throw when repositoryGroups is not an object', () => {
@@ -528,112 +667,233 @@ describe('BuildbotSyncer', () => {
                 const config = smallConfiguration();
                 config.repositoryGroups = 1;
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /repositoryGroups must specify a dictionary from the name to its definition/);
             assert.throws(() => {
                 const config = smallConfiguration();
                 config.repositoryGroups = 'hello';
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /repositoryGroups must specify a dictionary from the name to its definition/);
         });
 
         it('should throw when a repository group does not specify a dictionary of repositories', () => {
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.repositoryGroups = {'some-group': {'properties': {}}};
+                config.repositoryGroups = {'some-group': {testProperties: {}}};
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /Repository group "some-group" does not specify a dictionary of repositories/);
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.repositoryGroups = {'some-group': {'repositories': 1}, 'properties': {}};
+                config.repositoryGroups = {'some-group': {repositories: 1}, testProperties: {}};
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /Repository group "some-group" does not specify a dictionary of repositories/);
         });
 
         it('should throw when a repository group specifies an empty dictionary', () => {
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.repositoryGroups = {'some-group': {'repositories': {}, 'properties': {}}};
+                config.repositoryGroups = {'some-group': {repositories: {}, testProperties: {}}};
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /Repository group "some-group" does not specify any repository/);
         });
 
         it('should throw when a repository group specifies an invalid repository name', () => {
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.repositoryGroups = {'some-group': {'repositories': {'InvalidRepositoryName': {}}}};
+                config.repositoryGroups = {'some-group': {repositories: {'InvalidRepositoryName': {}}}};
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /"InvalidRepositoryName" is not a valid repository name/);
         });
 
         it('should throw when a repository group specifies a repository with a non-dictionary value', () => {
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.repositoryGroups = {'some-group': {'repositories': {'WebKit': 1}}};
+                config.repositoryGroups = {'some-group': {repositories: {'WebKit': 1}}};
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /"WebKit" specifies a non-dictionary value/);
         });
 
         it('should throw when the description of a repository group is not a string', () => {
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.repositoryGroups = {'some-group': {'repositories': [{'WebKit': {}}], 'description': 1}};
+                config.repositoryGroups = {'some-group': {repositories: {'WebKit': {}}, description: 1}};
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /Repository group "some-group" have an invalid description/);
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.repositoryGroups = {'some-group': {'repositories': [{'WebKit': {}}], 'description': [1, 2]}};
+                config.repositoryGroups = {'some-group': {repositories: {'WebKit': {}}, description: [1, 2]}};
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /Repository group "some-group" have an invalid description/);
         });
 
         it('should throw when a repository group does not specify a dictionary of properties', () => {
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.repositoryGroups = {'some-group': {'repositories': {'WebKit': {}}, properties: 1}};
+                config.repositoryGroups = {'some-group': {repositories: {'WebKit': {}}, testProperties: 1}};
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /Repository group "some-group" specifies the test configurations with an invalid type/);
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.repositoryGroups = {'some-group': {'repositories': {'WebKit': {}}, properties: 'hello'}};
+                config.repositoryGroups = {'some-group': {repositories: {'WebKit': {}}, testProperties: 'hello'}};
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /Repository group "some-group" specifies the test configurations with an invalid type/);
         });
 
         it('should throw when a repository group refers to a non-existent repository in the properties dictionary', () => {
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.repositoryGroups = {'some-group': {'repositories': {'WebKit': {}}, properties: {'wk': '<InvalidRepository>'}}};
+                config.repositoryGroups = {'some-group': {repositories: {'WebKit': {}}, testProperties: {'wk': {revision: 'InvalidRepository'}}}};
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /Repository group "some-group" an invalid repository "InvalidRepository"/);
         });
 
-        it('should throw when a repository group refers to a repository in the properties dictionary which is not listed in the list of repositories', () => {
+        it('should throw when a repository group refers to a repository which is not listed in the list of repositories', () => {
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.repositoryGroups = {'some-group': {'repositories': {'WebKit': {}}, properties: {'os': '<iOS>'}}};
+                config.repositoryGroups = {'some-group': {repositories: {'WebKit': {}}, testProperties: {'os': {revision: 'iOS'}}}};
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /Repository group "some-group" an invalid repository "iOS"/);
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.repositoryGroups = {'some-group': {
+                    repositories: {'WebKit': {acceptsPatch: true}},
+                    testProperties: {'wk': {revision: 'WebKit'}, 'install-roots': {'roots': {}}},
+                    buildProperties: {'os': {revision: 'iOS'}},
+                    acceptsRoots: true}};
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Repository group "some-group" an invalid repository "iOS"/);
+        });
+
+        it('should throw when a repository group refers to a repository in building a patch which does not accept a patch', () => {
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.repositoryGroups = {'some-group': {
+                    repositories: {'WebKit': {acceptsPatch: true}, 'iOS': {}},
+                    testProperties: {'wk': {revision: 'WebKit'}, 'ios': {revision: 'iOS'}, 'install-roots': {'roots': {}}},
+                    buildProperties: {'wk': {revision: 'WebKit'}, 'ios': {revision: 'iOS'}, 'wk-patch': {patch: 'iOS'}},
+                    acceptsRoots: true}};
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Repository group "some-group" specifies a patch for "iOS" but it does not accept a patch/);
+        });
+
+        it('should throw when a repository group specifies a patch without specifying a revision', () => {
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.repositoryGroups = {'some-group': {
+                    repositories: {'WebKit': {acceptsPatch: true}},
+                    testProperties: {'wk': {revision: 'WebKit'}, 'install-roots': {'roots': {}}},
+                    buildProperties: {'wk-patch': {patch: 'WebKit'}},
+                    acceptsRoots: true}};
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Repository group "some-group" specifies a patch for "WebKit" but does not specify a revision/);
         });
 
         it('should throw when a repository group does not use a listed repository', () => {
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.repositoryGroups = {'some-group': {'repositories': {'WebKit': {}}, properties: {}}};
+                config.repositoryGroups = {'some-group': {'repositories': {'WebKit': {}}, testProperties: {}}};
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /Repository group "some-group" does not use some of the repositories listed in testing/);
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.repositoryGroups = {'some-group': {
+                    repositories: {'WebKit': {acceptsPatch: true}},
+                    testProperties: {'wk': {revision: 'WebKit'}, 'install-roots': {'roots': {}}},
+                    buildProperties: {},
+                    acceptsRoots: true}};
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Repository group "some-group" does not use some of the repositories listed in building a patch/);
         });
 
         it('should throw when a repository group specifies non-boolean value to acceptsRoots', () => {
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.repositoryGroups = {'some-group': {'repositories': {'WebKit': {}}, 'properties': {'webkit': '<WebKit>'}, acceptsRoots: 1}};
+                config.repositoryGroups = {'some-group': {'repositories': {'WebKit': {}}, 'testProperties': {'webkit': {'revision': 'WebKit'}}, acceptsRoots: 1}};
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /Repository group "some-group" contains invalid acceptsRoots value:/);
             assert.throws(() => {
                 const config = smallConfiguration();
-                config.repositoryGroups = {'some-group': {'repositories': {'WebKit': {}}, 'properties': {'webkit': '<WebKit>'}, acceptsRoots: []}};
+                config.repositoryGroups = {'some-group': {'repositories': {'WebKit': {}}, 'testProperties': {'webkit': {'revision': 'WebKit'}}, acceptsRoots: []}};
                 BuildbotSyncer._loadConfig(MockRemoteAPI, config);
-            });
+            }, /Repository group "some-group" contains invalid acceptsRoots value:/);
+        });
+
+        it('should throw when a repository group specifies non-boolean value to acceptsPatch', () => {
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.repositoryGroups = {'some-group': {'repositories': {'WebKit': {acceptsPatch: 1}}, 'testProperties': {'webkit': {'revision': 'WebKit'}}}};
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /"WebKit" contains invalid acceptsPatch value:/);
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.repositoryGroups = {'some-group': {'repositories': {'WebKit': {acceptsPatch: []}}, 'testProperties': {'webkit': {'revision': 'WebKit'}}}};
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /"WebKit" contains invalid acceptsPatch value:/);
+        });
+
+        it('should throw when a repository group specifies a patch in testProperties', () => {
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.repositoryGroups = {'some-group': {'repositories': {'WebKit': {acceptsPatch: true}},
+                    'testProperties': {'webkit': {'revision': 'WebKit'}, 'webkit-patch': {'patch': 'WebKit'}}}};
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Repository group "some-group" specifies a patch for "WebKit" in the properties for testing/);
+        });
+
+        it('should throw when a repository group specifies roots in buildProperties', () => {
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.repositoryGroups = {'some-group': {
+                    repositories: {'WebKit': {acceptsPatch: true}},
+                    testProperties: {'webkit': {revision: 'WebKit'}, 'install-roots': {'roots': {}}},
+                    buildProperties: {'webkit': {revision: 'WebKit'}, 'patch': {patch: 'WebKit'}, 'install-roots': {roots: {}}},
+                    acceptsRoots: true}};
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Repository group "some-group" specifies roots in the properties for building/);
+        });
+
+        it('should throw when a repository group that does not accept roots specifies roots in testProperties', () => {
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.repositoryGroups = {'some-group': {
+                    repositories: {'WebKit': {}},
+                    testProperties: {'webkit': {'revision': 'WebKit'}, 'install-roots': {'roots': {}}}}};
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Repository group "some-group" specifies roots in a property but it does not accept roots/);
+        });
+
+        it('should throw when a repository group specifies buildProperties but does not accept roots', () => {
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.repositoryGroups = {'some-group': {
+                    repositories: {'WebKit': {acceptsPatch: true}},
+                    testProperties: {'webkit': {revision: 'WebKit'}},
+                    buildProperties: {'webkit': {revision: 'WebKit'}, 'webkit-patch': {patch: 'WebKit'}}}};
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Repository group "some-group" specifies the properties for building but does not accept roots in testing/);
+        });
+
+        it('should throw when a repository group specifies buildProperties but does not accept any patch', () => {
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.repositoryGroups = {'some-group': {
+                    repositories: {'WebKit': {}},
+                    testProperties: {'webkit': {'revision': 'WebKit'}, 'install-roots': {'roots': {}}},
+                    buildProperties: {'webkit': {'revision': 'WebKit'}},
+                    acceptsRoots: true}};
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Repository group "some-group" specifies the properties for building but does not accept any patches/);
+        });
+
+        it('should throw when a repository group accepts roots but does not specify roots in testProperties', () => {
+            assert.throws(() => {
+                const config = smallConfiguration();
+                config.repositoryGroups = {'some-group': {
+                    repositories: {'WebKit': {acceptsPatch: true}},
+                    testProperties: {'webkit': {revision: 'WebKit'}},
+                    buildProperties: {'webkit': {revision: 'WebKit'}, 'webkit-patch': {patch: 'WebKit'}},
+                    acceptsRoots: true}};
+                BuildbotSyncer._loadConfig(MockRemoteAPI, config);
+            }, /Repository group "some-group" accepts roots but does not specify roots in testProperties/);
         });
     });