- Revision
- 204151
- Author
- rn...@webkit.org
- Date
- 2016-08-04 16:24:14 -0700 (Thu, 04 Aug 2016)
Log Message
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.
Modified Paths
Diff
Modified: trunk/Websites/perf.webkit.org/ChangeLog (204150 => 204151)
--- trunk/Websites/perf.webkit.org/ChangeLog 2016-08-04 22:43:50 UTC (rev 204150)
+++ trunk/Websites/perf.webkit.org/ChangeLog 2016-08-04 23:24:14 UTC (rev 204151)
@@ -1,3 +1,45 @@
+2016-08-04 Ryosuke Niwa <rn...@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 <rn...@webkit.org>
REGRESSION: Tooltip for analysis tasks doesn't show up on charts
Modified: trunk/Websites/perf.webkit.org/tools/js/buildbot-syncer.js (204150 => 204151)
--- trunk/Websites/perf.webkit.org/tools/js/buildbot-syncer.js 2016-08-04 22:43:50 UTC (rev 204150)
+++ trunk/Websites/perf.webkit.org/tools/js/buildbot-syncer.js 2016-08-04 23:24:14 UTC (rev 204151)
@@ -262,47 +262,79 @@
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 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 builder = entry['builder'];
- if (builders[builder])
- this._validateAndMergeConfig(newConfig, builders[builder]);
+ let builder = entry['builder'];
+ if (builders[builder])
+ this._validateAndMergeConfig(config, 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');
+ this._createTestConfiguration(remote, syncerByBuilder, config);
+ }
+ }
- let test = Test.findByPath(newConfig.test);
- assert(test, `${newConfig.test} is not a valid test path`);
+ return Array.from(syncerByBuilder.values());
+ }
- let platform = Platform.findByName(newConfig.platform);
- assert(platform, `${newConfig.platform} is not a valid platform name`);
+ 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 syncer = syncerByBuilder.get(newConfig.builder);
- if (!syncer) {
- syncer = new BuildbotSyncer(remote, newConfig);
- syncerByBuilder.set(newConfig.builder, syncer);
- }
- syncer.addTestConfiguration(test, platform, newConfig.properties);
+ 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 Array.from(syncerByBuilder.values());
+ return configurations;
}
- static _validateAndMergeConfig(config, valuesToMerge)
+ 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'])
@@ -309,16 +341,14 @@
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 @@
assert(false, `Unrecognized parameter ${name}`);
}
}
+ return config;
}
static _validateAndMergeProperties(properties, configArguments)
Modified: trunk/Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js (204150 => 204151)
--- trunk/Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js 2016-08-04 22:43:50 UTC (rev 204150)
+++ trunk/Websites/perf.webkit.org/unit-tests/buildbot-syncer-tests.js 2016-08-04 23:24:14 UTC (rev 204151)
@@ -58,6 +58,58 @@
};
}
+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 @@
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 () {
Modified: trunk/Websites/perf.webkit.org/unit-tests/resources/mock-v3-models.js (204150 => 204151)
--- trunk/Websites/perf.webkit.org/unit-tests/resources/mock-v3-models.js 2016-08-04 22:43:50 UTC (rev 204150)
+++ trunk/Websites/perf.webkit.org/unit-tests/resources/mock-v3-models.js 2016-08-04 23:24:14 UTC (rev 204151)
@@ -31,8 +31,11 @@
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]});
});
