Syncing script's configuration duplicates a lot of boilerplate
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 Aug 2016 23:24:14 +0000 (23:24 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 Aug 2016 23:24:14 +0000 (23:24 +0000)
https://bugs.webkit.org/show_bug.cgi?id=160574

Rubber-stamped by Chris Dumez.

This patch makes each configuration accept an array of platforms and types so that we can write:

{"type": "speedometer", "builder": "mba", "platform": "Trunk El Capitan MacBookAir"},
{"type": "speedometer", "builder": "mbp", "platform": "Trunk El Capitan MacBookPro"},
{"type": "speedometer", "builder": "mba", "platform": "Trunk Sierra MacBookAir"},
{"type": "speedometer", "builder": "mbp", "platform": "Trunk Sierra MacBookPro"},
{"type": "jetstream", "builder": "mba", "platform": "Trunk El Capitan MacBookAir"},
{"type": "jetstream", "builder": "mbp", "platform": "Trunk El Capitan MacBookPro"},
{"type": "jetstream", "builder": "mba", "platform": "Trunk Sierra MacBookAir"},
{"type": "jetstream", "builder": "mbp", "platform": "Trunk Sierra MacBookPro"},

more concisely as:

{"builder": "mba", "types": ["speedometer", "jetstream"],
    "platforms": ["Trunk El Capitan MacBookAir", "Trunk Sierra MacBookAir"]},
{"builder": "mbp", "types": ["speedometer", "jetstream"],
    "platforms": ["Trunk El Capitan MacBookPro", "Trunk Sierra MacBookPro"]},

* tools/js/buildbot-syncer.js:
(BuildbotSyncer._loadConfig):
(BuildbotSyncer._expandTypesAndPlatforms): Added. Clones a new configuration entry for each type
and platform.
(BuildbotSyncer._createTestConfiguration): Extracted from _loadConfig.
(BuildbotSyncer._validateAndMergeConfig): Added a new argument that specifies a property that
shouldn't be merged into the configuration. Also added the support for 'types' and 'platforms',
and merged the code for verify an array of strings. Finally, allow the appearance of 'properties'
since this function can now be called on a cloned configuration in which 'arguments' had already
been renamed to 'properties'.

* unit-tests/buildbot-syncer-tests.js: Added a test case to parse a consolidated configuration.
(sampleiOSConfigWithExpansions): Added.

* unit-tests/resources/mock-v3-models.js:
(MockModels.inject): Added a few more mock models for the newly added test.

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/tools/js/buildbot-syncer.js
Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js
Websites/perf.webkit.org/unit-tests/resources/mock-v3-models.js

index 48b90a3..0373807 100644 (file)
@@ -1,3 +1,45 @@
+2016-08-04  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Syncing script's configuration duplicates a lot of boilerplate
+        https://bugs.webkit.org/show_bug.cgi?id=160574
+
+        Rubber-stamped by Chris Dumez.
+
+        This patch makes each configuration accept an array of platforms and types so that we can write:
+
+        {"type": "speedometer", "builder": "mba", "platform": "Trunk El Capitan MacBookAir"},
+        {"type": "speedometer", "builder": "mbp", "platform": "Trunk El Capitan MacBookPro"},
+        {"type": "speedometer", "builder": "mba", "platform": "Trunk Sierra MacBookAir"},
+        {"type": "speedometer", "builder": "mbp", "platform": "Trunk Sierra MacBookPro"},
+        {"type": "jetstream", "builder": "mba", "platform": "Trunk El Capitan MacBookAir"},
+        {"type": "jetstream", "builder": "mbp", "platform": "Trunk El Capitan MacBookPro"},
+        {"type": "jetstream", "builder": "mba", "platform": "Trunk Sierra MacBookAir"},
+        {"type": "jetstream", "builder": "mbp", "platform": "Trunk Sierra MacBookPro"},
+
+        more concisely as:
+
+        {"builder": "mba", "types": ["speedometer", "jetstream"],
+            "platforms": ["Trunk El Capitan MacBookAir", "Trunk Sierra MacBookAir"]},
+        {"builder": "mbp", "types": ["speedometer", "jetstream"],
+            "platforms": ["Trunk El Capitan MacBookPro", "Trunk Sierra MacBookPro"]},
+
+        * tools/js/buildbot-syncer.js:
+        (BuildbotSyncer._loadConfig):
+        (BuildbotSyncer._expandTypesAndPlatforms): Added. Clones a new configuration entry for each type
+        and platform.
+        (BuildbotSyncer._createTestConfiguration): Extracted from _loadConfig.
+        (BuildbotSyncer._validateAndMergeConfig): Added a new argument that specifies a property that
+        shouldn't be merged into the configuration. Also added the support for 'types' and 'platforms',
+        and merged the code for verify an array of strings. Finally, allow the appearance of 'properties'
+        since this function can now be called on a cloned configuration in which 'arguments' had already
+        been renamed to 'properties'.
+
+        * unit-tests/buildbot-syncer-tests.js: Added a test case to parse a consolidated configuration.
+        (sampleiOSConfigWithExpansions): Added.
+
+        * unit-tests/resources/mock-v3-models.js:
+        (MockModels.inject): Added a few more mock models for the newly added test.
+
 2016-07-26  Ryosuke Niwa  <rniwa@webkit.org>
 
         REGRESSION: Tooltip for analysis tasks doesn't show up on charts
index d1c7b8d..af60dea 100644 (file)
@@ -262,63 +262,93 @@ class BuildbotSyncer {
         for (let entry of config['configurations']) {
             let newConfig = {};
             this._validateAndMergeConfig(newConfig, shared);
-
             this._validateAndMergeConfig(newConfig, entry);
 
-            let type = entry['type'];
-            if (type) {
-                assert(types[type]);
-                this._validateAndMergeConfig(newConfig, types[type]);
-            }
-
-            let builder = entry['builder'];
-            if (builders[builder])
-                this._validateAndMergeConfig(newConfig, builders[builder]);
-
-            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');
-            assert('properties' in newConfig, 'configuration must specify arguments to post on a builder');
-            assert('buildRequestArgument' in newConfig, 'configuration must specify buildRequestArgument');
-
-            let test = Test.findByPath(newConfig.test);
-            assert(test, `${newConfig.test} is not a valid test path`);
+            let expandedConfigurations = this._expandTypesAndPlatforms(newConfig);
+            for (let config of expandedConfigurations) {
+                if ('type' in config) {
+                    let type = config['type'];
+                    assert(type, `${type} is not a valid type in the configuration`);
+                    this._validateAndMergeConfig(config, types[type]);
+                }
 
-            let platform = Platform.findByName(newConfig.platform);
-            assert(platform, `${newConfig.platform} is not a valid platform name`);
+                let builder = entry['builder'];
+                if (builders[builder])
+                    this._validateAndMergeConfig(config, builders[builder]);
 
-            let syncer = syncerByBuilder.get(newConfig.builder);
-            if (!syncer) {
-                syncer = new BuildbotSyncer(remote, newConfig);
-                syncerByBuilder.set(newConfig.builder, syncer);
+                this._createTestConfiguration(remote, syncerByBuilder, config);
             }
-            syncer.addTestConfiguration(test, platform, newConfig.properties);
         }
 
         return Array.from(syncerByBuilder.values());
     }
 
-    static _validateAndMergeConfig(config, valuesToMerge)
+    static _expandTypesAndPlatforms(unresolvedConfig)
+    {
+        let typeExpanded = [];
+        if ('types' in unresolvedConfig) {
+            for (let type of unresolvedConfig['types'])
+                typeExpanded.push(this._validateAndMergeConfig({'type': type}, unresolvedConfig, 'types'));
+        } else
+            typeExpanded.push(unresolvedConfig);
+
+        let 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)
+    {
+        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');
+        assert('properties' in newConfig, 'configuration must specify arguments to post on a builder');
+        assert('buildRequestArgument' in newConfig, 'configuration must specify buildRequestArgument');
+
+        let test = Test.findByPath(newConfig.test);
+        assert(test, `${newConfig.test} is not a valid test path`);
+
+        let platform = Platform.findByName(newConfig.platform);
+        assert(platform, `${newConfig.platform} is not a valid platform name`);
+
+        let syncer = syncerByBuilder.get(newConfig.builder);
+        if (!syncer) {
+            syncer = new BuildbotSyncer(remote, newConfig);
+            syncerByBuilder.set(newConfig.builder, syncer);
+        }
+        syncer.addTestConfiguration(test, platform, newConfig.properties);
+    }
+
+    static _validateAndMergeConfig(config, valuesToMerge, excludedProperty)
     {
         for (let name in valuesToMerge) {
             let value = valuesToMerge[name];
+            if (name == excludedProperty)
+                continue;
+
             switch (name) {
+            case 'properties': // fallthrough
             case 'arguments':
                 assert.equal(typeof(value), 'object', 'arguments should be a dictionary');
                 if (!config['properties'])
                     config['properties'] = {};
                 this._validateAndMergeProperties(config['properties'], value);
                 break;
-            case 'test':
-                assert(value instanceof Array, 'test should be an array');
-                assert(value.every(function (part) { return typeof part == 'string'; }), 'test should be an array of strings');
+            case 'test': // fallthrough
+            case 'slaveList': // fallthrough
+            case 'platforms':
+            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 'slaveList':
-                assert(value instanceof Array, 'slaveList should be an array');
-                assert(value.every(function (part) { return typeof part == 'string'; }), 'slaveList should be an array of strings');
-                config[name] = value;
-                break;
             case 'type': // fallthrough
             case 'builder': // fallthrough
             case 'platform': // fallthrough
@@ -331,6 +361,7 @@ class BuildbotSyncer {
                 assert(false, `Unrecognized parameter ${name}`);
             }
         }
+        return config;
     }
 
     static _validateAndMergeProperties(properties, configArguments)
index 118fde0..b45a5b7 100644 (file)
@@ -58,6 +58,58 @@ function sampleiOSConfig()
     };
 }
 
+function sampleiOSConfigWithExpansions()
+{
+    return {
+        "triggerableName": "build-webkit-ios",
+        "shared":
+            {
+                "arguments": {
+                    "webkit-revision": {"root": "WebKit"},
+                    "os-version": {"root": "iOS"}
+                },
+                "buildRequestArgument": "build-request-id"
+            },
+        "types": {
+            "iphone-plt": {
+                "test": ["PLT-iPhone"],
+                "arguments": {"test_name": "plt"}
+            },
+            "ipad-plt": {
+                "test": ["PLT-iPad"],
+                "arguments": {"test_name": "plt"}
+            },
+            "speedometer": {
+                "test": ["Speedometer"],
+                "arguments": {"tests": "speedometer"}
+            },
+        },
+        "builders": {
+            "iphone": {
+                "builder": "iPhone AB Tests",
+                "arguments": {"forcescheduler": "force-iphone-ab-tests"}
+            },
+            "ipad": {
+                "builder": "iPad AB Tests",
+                "arguments": {"forcescheduler": "force-ipad-ab-tests"}
+            },
+        },
+        "configurations": [
+            {
+                "builder": "iphone",
+                "platforms": ["iPhone", "iOS 10 iPhone"],
+                "types": ["iphone-plt", "speedometer"],
+            },
+            {
+                "builder": "ipad",
+                "platforms": ["iPad"],
+                "types": ["ipad-plt", "speedometer"],
+            },
+        ]
+    }
+    
+}
+
 let sampleRootSetData = {
     'WebKit': {
         'id': '111127',
@@ -482,6 +534,30 @@ describe('BuildbotSyncer', function () {
             assert.equal(configurations[1].platform, MockModels.ipad);
             assert.equal(configurations[1].test, MockModels.jetstream);
         });
+
+        it('should parse test configurations with types and platforms expansions correctly', function () {
+            let syncers = BuildbotSyncer._loadConfig(RemoteAPI, sampleiOSConfigWithExpansions());
+
+            assert.equal(syncers.length, 2);
+
+            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[3].platform, MockModels.iOS10iPhone);
+            assert.equal(configurations[3].test, MockModels.speedometer);
+
+            configurations = syncers[1].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);
+        });
     });
 
     describe('_propertiesForBuildRequest', function () {
index cae12f1..795ee1d 100644 (file)
@@ -31,8 +31,11 @@ var MockModels = {
 
             MockModels.iphone = Platform.ensureSingleton(12, {name: 'iPhone', metrics: []});
             MockModels.ipad = Platform.ensureSingleton(13, {name: 'iPad', metrics: []});
+            MockModels.iOS10iPhone = Platform.ensureSingleton(14, {name: 'iOS 10 iPhone', metrics: []});
 
             MockModels.plt = Test.ensureSingleton(844, {name: 'Page Load Test'});
+            MockModels.iPadPLT = Test.ensureSingleton(1444, {name: 'PLT-iPad'});
+            MockModels.iPhonePLT = Test.ensureSingleton(1500, {name: 'PLT-iPhone'});
             MockModels.pltMean = Metric.ensureSingleton(1158, {name: 'Time', aggregator: 'Arithmetic', test: MockModels.plt});
             MockModels.elCapitan = Platform.ensureSingleton(31, {name: 'El Capitan', metrics: [MockModels.pltMean]});
         });