Modernize BuildbotSyncer and BuildbotTriggerable
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 31 Mar 2017 02:11:32 +0000 (02:11 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 31 Mar 2017 02:11:32 +0000 (02:11 +0000)
https://bugs.webkit.org/show_bug.cgi?id=170310

Reviewed by Chris Dumez.

Modernized the code to use arrow functions and other modern idoms in ES2016.

* ReadMe.md: Added instructions on how to run tests, and moved the steps to configure postgres
above the steps to configure Apache since only the former is needed to run tests.
* tools/js/buildbot-syncer.js:
* tools/js/buildbot-triggerable.js:

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

Websites/perf.webkit.org/ChangeLog
Websites/perf.webkit.org/ReadMe.md
Websites/perf.webkit.org/tools/js/buildbot-syncer.js
Websites/perf.webkit.org/tools/js/buildbot-triggerable.js

index 6422732..0e8640c 100644 (file)
@@ -1,5 +1,19 @@
 2017-03-30  Ryosuke Niwa  <rniwa@webkit.org>
 
+        Modernize BuildbotSyncer and BuildbotTriggerable
+        https://bugs.webkit.org/show_bug.cgi?id=170310
+
+        Reviewed by Chris Dumez.
+
+        Modernized the code to use arrow functions and other modern idoms in ES2016.
+
+        * ReadMe.md: Added instructions on how to run tests, and moved the steps to configure postgres
+        above the steps to configure Apache since only the former is needed to run tests.
+        * tools/js/buildbot-syncer.js:
+        * tools/js/buildbot-triggerable.js:
+
+2017-03-30  Ryosuke Niwa  <rniwa@webkit.org>
+
         Yet another build fix after r214502. Workaround webkit.org/b/169907 for now.
 
         * public/v3/pages/analysis-task-page.js:
index 6cfc0e3..f02c738 100644 (file)
@@ -37,6 +37,46 @@ Then run `tools/remote-cache-server.py start`. This launches a httpd server on p
 The script caches remote server's responses under `public/data/remote-cache` and never revalidates them (to allow offline work).
 If you needed the latest content, delete caches stored in this directory by running `tools/remote-cache-server.py reset`.
 
+## Running Tests
+
+There are three kinds of tests in directories of the same name: `unit-tests`, `server tests`, and `browser-tests`.
+
+ - `unit-tests`: These tests various models and common JS code used in v3 UI and tools. They mock JSON APIs and model object.
+ - `server-tests`: Server tests use a real Apache server in accordance with `testServer` in `config.json` and a Postgres database by the name of `testDatabaseName` specified in `config.json`. They're functional tests and may test both the backend database schema, PHP, and corresponding front-end code although many of them directly queries and modifies the database.
+ - `browser-tests`: These are tests to be ran inside a Web browser, and tests v3 UI's interaction with browser's DOM API.
+
+To run `unit-tests` and `server-tests`, simply run `./tools/run-tests.py` after installing dependencies and configuring the PostgreSQL.
+To run `browser-tests`, open `browser-tests/index.html` inside a Web browser.
+
+## Configuring PostgreSQL
+
+Run the following command to setup a Postgres server at `/Volumes/Data/perf.webkit.org/PostgresSQL` (or wherever you'd prefer):
+`python ./tools/setup-database.py /Volumes/Data/perf.webkit.org/PostgresSQL`
+
+It automatically retrieves the database name, the username, and the password from `config.json`.
+
+### Starting PostgreSQL
+
+The setup script automatically starts the database but you may need to run the following command to manually start the database after reboot.
+
+- Starting the database: `/Applications/Server.app/Contents/ServerRoot/usr/bin/pg_ctl -D /Volumes/Data/perf.webkit.org/PostgresSQL -l /Volumes/Data/perf.webkit.org/PostgresSQL/logfile -o "-k /Volumes/Data/perf.webkit.org/PostgresSQL" start`
+- Stopping the database: `/Applications/Server.app/Contents/ServerRoot/usr/bin/pg_ctl -D /Volumes/Data/perf.webkit.org/PostgresSQL -l /Volumes/Data/perf.webkit.org/PostgresSQL/logfile -o "-k /Volumes/Data/perf.webkit.org/PostgresSQL" stop`
+
+### Initializing the Database
+
+Run `database/init-database.sql` in psql as `webkit-perf-db-user`:
+`/Applications/Server.app/Contents/ServerRoot/usr/bin/psql webkit-perf-db -h localhost --username webkit-perf-db-user -f init-database.sql`
+
+### Making a Backup and Restoring
+
+Use `pg_dump` and `pg_restore` to backup and restore the database. If you're replicating the production database for development purposes, you may consider excluding `run_iterations` table, which takes up 2/3 of the storage space, to reduce the size of the database for your local copy. Adjust the number of concurrent processes to use by `--jobs` and adjust the compression level by `--compress` (0 is no compression, 9 is most compressed).
+
+- Making the fullbackup of the database: `/Applications/Server.app/Contents/ServerRoot/usr/bin/pg_dump -h localhost webkit-perf-db --format=directory --file=<path to backup directory> --jobs=4 --no-owner --compress=7`
+
+- Making an abridged backup without `run_iterations` table: `/Applications/Server.app/Contents/ServerRoot/usr/bin/pg_dump -h localhost webkit-perf-db --format=directory --file=<path to backup directory> --jobs=4 --no-owner --compress=7 --exclude-table=run_iterations`
+
+- Restoring the database: `/Applications/Server.app/Contents/ServerRoot/usr/bin/pg_restore --format=directory --jobs=4 --no-owner --host localhost --username=webkit-perf-db-user <path to backup directory> --dbname=webkit-perf-db`
+    
 ## Configuring Apache
 
 ### Instructions if you're using Server.app
@@ -109,35 +149,6 @@ AuthUserFile "<Realm>"
 Require valid-user
 ```
 
-## Configuring PostgreSQL
-
-Run the following command to setup a Postgres server at `/Volumes/Data/perf.webkit.org/PostgresSQL` (or wherever you'd prefer):
-`python ./tools/setup-database.py /Volumes/Data/perf.webkit.org/PostgresSQL`
-
-It automatically retrieves the database name, the username, and the password from `config.json`.
-
-### Starting PostgreSQL
-
-The setup script automatically starts the database but you may need to run the following command to manually start the database after reboot.
-
-- Starting the database: `/Applications/Server.app/Contents/ServerRoot/usr/bin/pg_ctl -D /Volumes/Data/perf.webkit.org/PostgresSQL -l /Volumes/Data/perf.webkit.org/PostgresSQL/logfile -o "-k /Volumes/Data/perf.webkit.org/PostgresSQL" start`
-- Stopping the database: `/Applications/Server.app/Contents/ServerRoot/usr/bin/pg_ctl -D /Volumes/Data/perf.webkit.org/PostgresSQL -l /Volumes/Data/perf.webkit.org/PostgresSQL/logfile -o "-k /Volumes/Data/perf.webkit.org/PostgresSQL" stop`
-
-### Initializing the Database
-
-Run `database/init-database.sql` in psql as `webkit-perf-db-user`:
-`/Applications/Server.app/Contents/ServerRoot/usr/bin/psql webkit-perf-db -h localhost --username webkit-perf-db-user -f init-database.sql`
-
-### Making a Backup and Restoring
-
-Use `pg_dump` and `pg_restore` to backup and restore the database. If you're replicating the production database for development purposes, you may consider excluding `run_iterations` table, which takes up 2/3 of the storage space, to reduce the size of the database for your local copy. Adjust the number of concurrent processes to use by `--jobs` and adjust the compression level by `--compress` (0 is no compression, 9 is most compressed).
-
-- Making the fullbackup of the database: `/Applications/Server.app/Contents/ServerRoot/usr/bin/pg_dump -h localhost webkit-perf-db --format=directory --file=<path to backup directory> --jobs=4 --no-owner --compress=7`
-
-- Making an abridged backup without `run_iterations` table: `/Applications/Server.app/Contents/ServerRoot/usr/bin/pg_dump -h localhost webkit-perf-db --format=directory --file=<path to backup directory> --jobs=4 --no-owner --compress=7 --exclude-table=run_iterations`
-
-- Restoring the database: `/Applications/Server.app/Contents/ServerRoot/usr/bin/pg_restore --format=directory --jobs=4 --no-owner --host localhost --username=webkit-perf-db-user <path to backup directory> --dbname=webkit-perf-db`
-
 ## Concepts
 
  - **Test** - A test is a collection of metrics such as frame rate and malloced bytes. It can have a children and a parent and form a tree of tests.
index 6de521c..94b5a20 100644 (file)
@@ -17,8 +17,8 @@ class BuildbotBuildEntry {
 
         for (let propertyTuple of (rawData['properties'] || [])) {
             // e.g. ['build_request_id', '16733', 'Force Build Form']
-            let name = propertyTuple[0];
-            let value = propertyTuple[1];
+            const name = propertyTuple[0];
+            const value = propertyTuple[1];
             if (name == syncer._slavePropertyName)
                 this._slaveName = value;
             else if (name == syncer._buildRequestPropertyName)
@@ -75,17 +75,13 @@ class BuildbotSyncer {
     {
         assert(test instanceof Test);
         assert(platform instanceof Platform);
-        this._testConfigurations.push({test: test, platform: platform, propertiesTemplate: propertiesTemplate});
+        this._testConfigurations.push({test, platform, propertiesTemplate});
     }
     testConfigurations() { return this._testConfigurations; }
 
     matchesConfiguration(request)
     {
-        for (let config of this._testConfigurations) {
-            if (config.platform == request.platform() && config.test == request.test())
-                return true;
-        }
-        return false;
+        return this._testConfigurations.some((config) => config.platform == request.platform() && config.test == request.test());
     }
 
     scheduleRequest(newRequest, slaveName)
@@ -140,10 +136,9 @@ class BuildbotSyncer {
 
     pullBuildbot(count)
     {
-        let self = this;
-        return this._remote.getJSON(this.pathForPendingBuildsJSON()).then(function (content) {
-            let pendingEntries = content.map(function (entry) { return new BuildbotBuildEntry(self, entry); });
-            return self._pullRecentBuilds(count).then(function (entries) {
+        return this._remote.getJSON(this.pathForPendingBuildsJSON()).then((content) => {
+            let pendingEntries = content.map((entry) => new BuildbotBuildEntry(this, entry));
+            return this._pullRecentBuilds(count).then((entries) => {
                 let entryByRequest = {};
 
                 for (let entry of pendingEntries)
@@ -156,8 +151,8 @@ class BuildbotSyncer {
                 for (let id in entryByRequest)
                     entryList.push(entryByRequest[id]);
 
-                self._entryList = entryList;
-                self._slavesWithNewRequests.clear();
+                this._entryList = entryList;
+                this._slavesWithNewRequests.clear();
 
                 return entryList;
             });
@@ -173,13 +168,12 @@ class BuildbotSyncer {
         for (let i = 0; i < count; i++)
             selectedBuilds[i] = -i - 1;
 
-        let self = this;
-        return this._remote.getJSON(this.pathForBuildJSON(selectedBuilds)).then(function (content) {
-            var entries = [];
+        return this._remote.getJSON(this.pathForBuildJSON(selectedBuilds)).then((content) => {
+            const entries = [];
             for (let index of selectedBuilds) {
-                let entry = content[index];
+                const entry = content[index];
                 if (entry && !entry['error'])
-                    entries.push(new BuildbotBuildEntry(self, entry));
+                    entries.push(new BuildbotBuildEntry(this, entry));
             }
             return entries;
         });
@@ -188,8 +182,7 @@ class BuildbotSyncer {
     pathForPendingBuildsJSON() { return `/json/builders/${escape(this._builderName)}/pendingBuilds`; }
     pathForBuildJSON(selectedBuilds)
     {
-        return `/json/builders/${escape(this._builderName)}/builds/?`
-            + selectedBuilds.map(function (number) { return 'select=' + number; }).join('&');
+        return `/json/builders/${escape(this._builderName)}/builds/?` + selectedBuilds.map((number) => 'select=' + number).join('&');
     }
     pathForForceBuild() { return `/builders/${escape(this._builderName)}/force`; }
 
@@ -214,14 +207,14 @@ class BuildbotSyncer {
         }
         assert(propertiesTemplate);
 
-        let properties = {};
+        const properties = {};
         for (let key in propertiesTemplate) {
-            let value = propertiesTemplate[key];
+            const value = propertiesTemplate[key];
             if (typeof(value) != 'object')
                 properties[key] = value;
             else if ('root' in value) {
-                let repositoryName = value['root'];
-                let repository = repositoryByName[repositoryName];
+                const repositoryName = value['root'];
+                const repository = repositoryByName[repositoryName];
                 assert(repository, `"${repositoryName}" must be specified`);
                 properties[key] = commitSet.revisionForRepository(repository);
             } else if ('rootOptions' in value) {
@@ -230,7 +223,7 @@ class BuildbotSyncer {
                 properties[key] = commitSet.revisionForRepository(repositoryByName[filteredOptions[0]]);
             }
             else if ('rootsExcluding' in value) {
-                let revisionSet = this._revisionSetFromCommitSetWithExclusionList(commitSet, value['rootsExcluding']);
+                const revisionSet = this._revisionSetFromCommitSetWithExclusionList(commitSet, value['rootsExcluding']);
                 properties[key] = JSON.stringify(revisionSet);
             }
         }
@@ -259,25 +252,25 @@ class BuildbotSyncer {
 
     static _loadConfig(remote, config)
     {
-        let shared = config['shared'] || {};
-        let types = config['types'] || {};
-        let builders = config['builders'] || {};
+        const shared = config['shared'] || {};
+        const types = config['types'] || {};
+        const builders = config['builders'] || {};
 
         let syncerByBuilder = new Map;
         for (let entry of config['configurations']) {
-            let newConfig = {};
+            const newConfig = {};
             this._validateAndMergeConfig(newConfig, shared);
             this._validateAndMergeConfig(newConfig, entry);
 
-            let expandedConfigurations = this._expandTypesAndPlatforms(newConfig);
+            const expandedConfigurations = this._expandTypesAndPlatforms(newConfig);
             for (let config of expandedConfigurations) {
                 if ('type' in config) {
-                    let type = config['type'];
+                    const type = config['type'];
                     assert(type, `${type} is not a valid type in the configuration`);
                     this._validateAndMergeConfig(config, types[type]);
                 }
 
-                let builder = entry['builder'];
+                const builder = entry['builder'];
                 if (builders[builder])
                     this._validateAndMergeConfig(config, builders[builder]);
 
@@ -290,14 +283,14 @@ class BuildbotSyncer {
 
     static _expandTypesAndPlatforms(unresolvedConfig)
     {
-        let typeExpanded = [];
+        const 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 = [];
+        const configurations = [];
         for (let config of typeExpanded) {
             if ('platforms' in config) {
                 for (let platform of config['platforms'])
@@ -317,10 +310,10 @@ class BuildbotSyncer {
         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);
+        const test = Test.findByPath(newConfig.test);
         assert(test, `${newConfig.test} is not a valid test path`);
 
-        let platform = Platform.findByName(newConfig.platform);
+        const platform = Platform.findByName(newConfig.platform);
         assert(platform, `${newConfig.platform} is not a valid platform name`);
 
         let syncer = syncerByBuilder.get(newConfig.builder);
@@ -333,8 +326,8 @@ class BuildbotSyncer {
 
     static _validateAndMergeConfig(config, valuesToMerge, excludedProperty)
     {
-        for (let name in valuesToMerge) {
-            let value = valuesToMerge[name];
+        for (const name in valuesToMerge) {
+            const value = valuesToMerge[name];
             if (name == excludedProperty)
                 continue;
 
@@ -372,14 +365,14 @@ class BuildbotSyncer {
     static _validateAndMergeProperties(properties, configArguments)
     {
         for (let name in configArguments) {
-            let value = configArguments[name];
+            const value = configArguments[name];
             if (typeof(value) == 'string') {
                 properties[name] = value;
                 continue;
             }
             assert.equal(typeof(value), 'object', 'A argument value must be either a string or a dictionary');
 
-            let keys = Object.keys(value);
+            const keys = Object.keys(value);
             assert.equal(keys.length, 1, 'arguments value cannot contain more than one key');
             let namedValue = value[keys[0]];
             switch (keys[0]) {
index 45b22c1..c1274d6 100644 (file)
@@ -22,7 +22,7 @@ class BuildbotTriggerable {
         assert(typeof(slaveInfo.password) == 'string', 'slave password must be specified');
 
         this._syncers = BuildbotSyncer._loadConfig(buildbotRemote, config);
-        this._logger = logger || {log: function () { }, error: function () { }};
+        this._logger = logger || {log: () => { }, error: () => { }};
     }
 
     name() { return this._name; }
@@ -48,40 +48,39 @@ class BuildbotTriggerable {
         let syncerList = this._syncers;
         let buildReqeustsByGroup = new Map;
 
-        let self = this;
         this._logger.log(`Fetching build requests for ${this._name}...`);
-        return BuildRequest.fetchForTriggerable(this._name).then(function (buildRequests) {
-            self._validateRequests(buildRequests);
+        return BuildRequest.fetchForTriggerable(this._name).then((buildRequests) => {
+            this._validateRequests(buildRequests);
             buildReqeustsByGroup = BuildbotTriggerable._testGroupMapForBuildRequests(buildRequests);
-            return self._pullBuildbotOnAllSyncers(buildReqeustsByGroup);
-        }).then(function (updates) {
-            self._logger.log('Scheduling builds');
-            let promistList = [];
-            let testGroupList = Array.from(buildReqeustsByGroup.values()).sort(function (a, b) { return a.groupOrder - b.groupOrder; });
-            for (let group of testGroupList) {
-                let promise = self._scheduleNextRequestInGroupIfSlaveIsAvailable(group, updates);
+            return this._pullBuildbotOnAllSyncers(buildReqeustsByGroup);
+        }).then((updates) => {
+            this._logger.log('Scheduling builds');
+            const promistList = [];
+            const testGroupList = Array.from(buildReqeustsByGroup.values()).sort(function (a, b) { return a.groupOrder - b.groupOrder; });
+            for (const group of testGroupList) {
+                const promise = this._scheduleNextRequestInGroupIfSlaveIsAvailable(group, updates);
                 if (promise)
                     promistList.push(promise);
             }
             return Promise.all(promistList);
-        }).then(function () {
+        }).then(() => {
             // Pull all buildbots for the second time since the previous step may have scheduled more builds.
-            return self._pullBuildbotOnAllSyncers(buildReqeustsByGroup);
-        }).then(function (updates) {
+            return this._pullBuildbotOnAllSyncers(buildReqeustsByGroup);
+        }).then((updates) => {
             // FIXME: Add a new API that just updates the requests.
-            return self._remote.postJSONWithStatus(`/api/build-requests/${self._name}`, {
-                'slaveName': self._slaveInfo.name,
-                'slavePassword': self._slaveInfo.password,
+            return this._remote.postJSONWithStatus(`/api/build-requests/${this._name}`, {
+                'slaveName': this._slaveInfo.name,
+                'slavePassword': this._slaveInfo.password,
                 'buildRequestUpdates': updates});
         });
     }
 
     _validateRequests(buildRequests)
     {
-        let testPlatformPairs = {};
+        const testPlatformPairs = {};
         for (let request of buildRequests) {
-            if (!this._syncers.some(function (syncer) { return syncer.matchesConfiguration(request); })) {
-                let key = request.platform().id + '-' + request.test().id();
+            if (!this._syncers.some((syncer) => syncer.matchesConfiguration(request))) {
+                const key = request.platform().id + '-' + request.test().id();
                 if (!(key in testPlatformPairs))
                     this._logger.error(`No matching configuration for "${request.test().fullName()}" on "${request.platform().name()}".`);                
                 testPlatformPairs[key] = true;
@@ -93,16 +92,15 @@ class BuildbotTriggerable {
     {
         let updates = {};
         let associatedRequests = new Set;
-        let self = this;
-        return Promise.all(this._syncers.map(function (syncer) {
-            return syncer.pullBuildbot(self._lookbackCount).then(function (entryList) {
-                for (let entry of entryList) {
-                    let request = BuildRequest.findById(entry.buildRequestId());
+        return Promise.all(this._syncers.map((syncer) => {
+            return syncer.pullBuildbot(this._lookbackCount).then((entryList) => {
+                for (const entry of entryList) {
+                    const request = BuildRequest.findById(entry.buildRequestId());
                     if (!request)
                         continue;
                     associatedRequests.add(request);
 
-                    let info = buildReqeustsByGroup.get(request.testGroupId());
+                    const info = buildReqeustsByGroup.get(request.testGroupId());
                     assert(!info.syncer || info.syncer == syncer);
                     info.syncer = syncer;
                     if (entry.slaveName()) {
@@ -110,31 +108,31 @@ class BuildbotTriggerable {
                         info.slaveName = entry.slaveName();
                     }
 
-                    let newStatus = entry.buildRequestStatusIfUpdateIsNeeded(request);
+                    const newStatus = entry.buildRequestStatusIfUpdateIsNeeded(request);
                     if (newStatus) {
-                        self._logger.log(`Updating the status of build request ${request.id()} from ${request.status()} to ${newStatus}`);
+                        this._logger.log(`Updating the status of build request ${request.id()} from ${request.status()} to ${newStatus}`);
                         updates[entry.buildRequestId()] = {status: newStatus, url: entry.url()};
                     } else if (!request.statusUrl()) {
-                        self._logger.log(`Setting the status URL of build request ${request.id()} to ${entry.url()}`);
+                        this._logger.log(`Setting the status URL of build request ${request.id()} to ${entry.url()}`);
                         updates[entry.buildRequestId()] = {status: request.status(), url: entry.url()};
                     }
                 }
             });
-        })).then(function () {
-            for (let request of BuildRequest.all()) {
+        })).then(() => {
+            for (const request of BuildRequest.all()) {
                 if (request.hasStarted() && !request.hasFinished() && !associatedRequests.has(request)) {
-                    self._logger.log(`Updating the status of build request ${request.id()} from ${request.status()} to failedIfNotCompleted`);
+                    this._logger.log(`Updating the status of build request ${request.id()} from ${request.status()} to failedIfNotCompleted`);
                     assert(!(request.id() in updates));
                     updates[request.id()] = {status: 'failedIfNotCompleted'};
                 }
             }
-        }).then(function () { return updates; });
+        }).then(() => updates);
     }
 
     _scheduleNextRequestInGroupIfSlaveIsAvailable(groupInfo, pendingUpdates)
     {
         let nextRequest = null;
-        for (let request of groupInfo.requests) {
+        for (const request of groupInfo.requests) {
             if (request.isScheduled() || (request.id() in pendingUpdates && pendingUpdates[request.id()]['status'] == 'scheduled'))
                 break;
             if (request.isPending() && !(request.id() in pendingUpdates)) {
@@ -157,6 +155,7 @@ class BuildbotTriggerable {
 
         if (!syncer) {
             for (syncer of this._syncers) {
+                // FIXME: This shouldn't be a new variable.
                 let promise = syncer.scheduleRequestInGroupIfAvailable(nextRequest);
                 if (promise)
                     break;
@@ -174,7 +173,7 @@ class BuildbotTriggerable {
 
     static _testGroupMapForBuildRequests(buildRequests)
     {
-        let map = new Map;
+        const map = new Map;
         let groupOrder = 0;
         for (let request of buildRequests) {
             let groupId = request.testGroupId();