From ad8d2552f5182552a85d7f5734a41d0bd79aa39a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Raphael=20von=20der=20Gr=C3=BCn?= Date: Wed, 9 Oct 2019 14:03:51 +0200 Subject: [PATCH 1/3] Cleanup HooksRunner code --- src/hooks/HooksRunner.js | 79 +++++++++++++++++----------------------- 1 file changed, 33 insertions(+), 46 deletions(-) diff --git a/src/hooks/HooksRunner.js b/src/hooks/HooksRunner.js index c1248343a..a42044f25 100644 --- a/src/hooks/HooksRunner.js +++ b/src/hooks/HooksRunner.js @@ -53,14 +53,11 @@ HooksRunner.prototype.fire = function fire (hook, opts) { } opts = this.prepareOptions(opts); - var scripts = scriptsFinder.getHookScripts(hook, opts); - - // execute hook event listeners first - return executeEventHandlersSerially(hook, opts).then(function () { - // then execute hook script files - var context = new Context(hook, opts); - return runScriptsSerially(scripts, context); - }); + // first run hook event listeners, then run hook script files + return runJobsSerially([ + ...getEventHandlerJobs(hook, opts), + ...getHookJobs(hook, opts) + ]); }; /** @@ -96,35 +93,27 @@ function globalFire (hook, opts) { } opts = opts || {}; - return executeEventHandlersSerially(hook, opts); + return runJobsSerially(getEventHandlerJobs(hook, opts)); } -// Returns a promise. -function executeEventHandlersSerially (hook, opts) { - var handlers = events.listeners(hook); - if (handlers.length) { - // Chain the handlers in series. - return handlers.reduce(function (soFar, f) { - return soFar.then(function () { return f(opts); }); - }, Promise.resolve()); - } else { - return Promise.resolve(); // Nothing to do. - } +function getEventHandlerJobs (hook, opts) { + return events.listeners(hook) + .map(handler => () => handler(opts)); } -/** - * Serially fires scripts either via Promise.resolve(require(pathToScript)(context)) or via child_process.spawn. - * Returns promise. - */ -function runScriptsSerially (scripts, context) { +function getHookJobs (hook, opts) { + const scripts = scriptsFinder.getHookScripts(hook, opts); + if (scripts.length === 0) { - events.emit('verbose', 'No scripts found for hook "' + context.hook + '".'); + events.emit('verbose', `No scripts found for hook "${hook}".`); } - return scripts.reduce(function (prevScriptPromise, nextScript) { - return prevScriptPromise.then(function () { - return runScript(nextScript, context); - }); - }, Promise.resolve()); + + const context = new Context(hook, opts); + return scripts.map(script => () => runScript(script, context)); +} + +function runJobsSerially (jobs) { + return jobs.reduce((acc, job) => acc.then(job), Promise.resolve()); } /** @@ -207,13 +196,18 @@ function runScriptViaChildProcessSpawn (script, context) { } } - var execOpts = { cwd: opts.projectRoot, printCommand: true, stdio: 'inherit' }; - execOpts.env = {}; - execOpts.env.CORDOVA_VERSION = require('../../package').version; - execOpts.env.CORDOVA_PLATFORMS = opts.platforms ? opts.platforms.join() : ''; - execOpts.env.CORDOVA_PLUGINS = opts.plugins ? opts.plugins.join() : ''; - execOpts.env.CORDOVA_HOOK = script.fullPath; - execOpts.env.CORDOVA_CMDLINE = process.argv.join(' '); + const execOpts = { + cwd: opts.projectRoot, + printCommand: true, + stdio: 'inherit', + env: { + CORDOVA_VERSION: require('../../package').version, + CORDOVA_PLATFORMS: opts.platforms ? opts.platforms.join() : '', + CORDOVA_PLUGINS: opts.plugins ? opts.plugins.join() : '', + CORDOVA_HOOK: script.fullPath, + CORDOVA_CMDLINE: process.argv.join(' ') + } + }; return superspawn.spawn(command, args, execOpts) .catch(function (err) { @@ -251,12 +245,5 @@ function isHookDisabled (opts, hook) { if (opts === undefined || opts.nohooks === undefined) { return false; } - var disabledHooks = opts.nohooks; - var length = disabledHooks.length; - for (var i = 0; i < length; i++) { - if (hook.match(disabledHooks[i]) !== null) { - return true; - } - } - return false; + return opts.nohooks.some(pattern => hook.match(pattern) !== null); } From 09ca3a6a7469d2822f01eb33c40dacfc537a9f51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Raphael=20von=20der=20Gr=C3=BCn?= Date: Wed, 9 Oct 2019 17:09:42 +0200 Subject: [PATCH 2/3] Merge the 'plugin hooks' blocks in HooksRunner.spec --- integration-tests/HooksRunner.spec.js | 31 ++++++++++++--------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/integration-tests/HooksRunner.spec.js b/integration-tests/HooksRunner.spec.js index f7a89e4ac..f823b4422 100644 --- a/integration-tests/HooksRunner.spec.js +++ b/integration-tests/HooksRunner.spec.js @@ -183,6 +183,20 @@ describe('HooksRunner', function () { }); describe('plugin hooks', function () { + it('Test 009 : should execute hook scripts serially from plugin.xml', function () { + usePluginConfig('OnlyNonPlatformScripts'); + + return hooksRunner.fire(test_event, hookOptions) + .then(checkHooksOrderFile); + }); + + it('Test 010 : should execute hook scripts serially from plugin.xml including platform scripts', function () { + usePluginConfig('OnePlatform'); + + return hooksRunner.fire(test_event, hookOptions) + .then(checkHooksOrderFile); + }); + it('Test 011 : should filter hook scripts from plugin.xml by platform', function () { // Make scripts executable globby.sync('scripts/**', { cwd: testPluginInstalledPath, absolute: true }) @@ -249,23 +263,6 @@ describe('HooksRunner', function () { }); }); }, 20 * 1000); - }); - - describe('plugin hooks', function () { - - it('Test 009 : should execute hook scripts serially from plugin.xml', function () { - usePluginConfig('OnlyNonPlatformScripts'); - - return hooksRunner.fire(test_event, hookOptions) - .then(checkHooksOrderFile); - }); - - it('Test 010 : should execute hook scripts serially from plugin.xml including platform scripts', function () { - usePluginConfig('OnePlatform'); - - return hooksRunner.fire(test_event, hookOptions) - .then(checkHooksOrderFile); - }); it('Test 013 : should not execute the designated hook when --nohooks option specifies the exact hook name', function () { hookOptions.nohooks = ['before_build']; From 965d534e4a370ec5c930f1e098b904f737134308 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Raphael=20von=20der=20Gr=C3=BCn?= Date: Wed, 9 Oct 2019 16:26:07 +0200 Subject: [PATCH 3/3] Reduce amount of fixtures for HooksRunner test --- integration-tests/HooksRunner.spec.js | 95 ++++++++++++++++--- .../pluginOnePlatform_bat.xml | 19 ---- .../pluginOnePlatform_sh.xml | 19 ---- .../pluginOnlyNonPlatformScripts_bat.xml | 14 --- .../pluginOnlyNonPlatformScripts_sh.xml | 14 --- .../pluginTwoPlatforms_bat.xml | 24 ----- .../pluginTwoPlatforms_sh.xml | 24 ----- .../projWithHooks/configOnePlatform_bat.xml | 22 ----- .../projWithHooks/configOnePlatform_sh.xml | 22 ----- .../configOnlyNonPlatformScripts_bat.xml | 16 ---- .../configOnlyNonPlatformScripts_sh.xml | 16 ---- .../projWithHooks/configTwoPlatforms_bat.xml | 28 ------ .../projWithHooks/configTwoPlatforms_sh.xml | 28 ------ 13 files changed, 84 insertions(+), 257 deletions(-) delete mode 100644 spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginOnePlatform_bat.xml delete mode 100644 spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginOnePlatform_sh.xml delete mode 100644 spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginOnlyNonPlatformScripts_bat.xml delete mode 100644 spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginOnlyNonPlatformScripts_sh.xml delete mode 100644 spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginTwoPlatforms_bat.xml delete mode 100644 spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginTwoPlatforms_sh.xml delete mode 100644 spec/cordova/fixtures/projWithHooks/configOnePlatform_bat.xml delete mode 100644 spec/cordova/fixtures/projWithHooks/configOnePlatform_sh.xml delete mode 100644 spec/cordova/fixtures/projWithHooks/configOnlyNonPlatformScripts_bat.xml delete mode 100644 spec/cordova/fixtures/projWithHooks/configOnlyNonPlatformScripts_sh.xml delete mode 100644 spec/cordova/fixtures/projWithHooks/configTwoPlatforms_bat.xml delete mode 100644 spec/cordova/fixtures/projWithHooks/configTwoPlatforms_sh.xml diff --git a/integration-tests/HooksRunner.spec.js b/integration-tests/HooksRunner.spec.js index f823b4422..be42bf651 100644 --- a/integration-tests/HooksRunner.spec.js +++ b/integration-tests/HooksRunner.spec.js @@ -21,12 +21,13 @@ const path = require('path'); const fs = require('fs-extra'); const delay = require('delay'); const globby = require('globby'); +const et = require('elementtree'); const HooksRunner = require('../src/hooks/HooksRunner'); const cordovaUtil = require('../src/cordova/util'); const cordova = require('../src/cordova/cordova'); const { tmpDir, testPlatform } = require('../spec/helpers'); -const { PluginInfo } = require('cordova-common'); +const { PluginInfo, ConfigParser } = require('cordova-common'); const { Q_chainmap } = require('../src/util/promise-util'); const tmp = tmpDir('hooks_test'); @@ -131,12 +132,43 @@ describe('HooksRunner', function () { expect(hooksOrder).toEqual(sortedHooksOrder); } - function useAppConfig (name) { - fs.copySync(path.join(project, `config${name}_${ext}.xml`), path.join(project, 'config.xml')); + const BASE_HOOKS = ` + + + + + + `; + const WINDOWS_HOOKS = ` + + + + + + + + `; + const ANDROID_HOOKS = ` + + + + + + + + `; + + function addHooks (hooksXml, doc) { + const hooks = et.parse(hooksXml); + for (const el of hooks.getroot().findall('./*')) { + doc.getroot().append(el); + } } - function usePluginConfig (name) { - fs.copySync(path.join(testPluginInstalledPath, `plugin${name}_${ext}.xml`), path.join(testPluginInstalledPath, 'plugin.xml')); + function addHooksToConfig (hooksXml) { + const config = new ConfigParser(path.join(project, 'config.xml')); + addHooks(hooksXml, config.doc); + config.write(); } describe('application hooks', function () { @@ -154,21 +186,24 @@ describe('HooksRunner', function () { }); it('Test 006 : should execute hook scripts serially from config.xml', function () { - useAppConfig('OnlyNonPlatformScripts'); + addHooksToConfig(BASE_HOOKS); return hooksRunner.fire(test_event, hookOptions) .then(checkHooksOrderFile); }); it('Test 007 : should execute hook scripts serially from config.xml including platform scripts', function () { - useAppConfig('OnePlatform'); + addHooksToConfig(BASE_HOOKS); + addHooksToConfig(WINDOWS_HOOKS); return hooksRunner.fire(test_event, hookOptions) .then(checkHooksOrderFile); }); it('Test 008 : should filter hook scripts from config.xml by platform', function () { - useAppConfig('TwoPlatforms'); + addHooksToConfig(BASE_HOOKS); + addHooksToConfig(WINDOWS_HOOKS); + addHooksToConfig(ANDROID_HOOKS); hookOptions.cordova.platforms = ['android']; return hooksRunner.fire(test_event, hookOptions).then(function () { @@ -183,15 +218,51 @@ describe('HooksRunner', function () { }); describe('plugin hooks', function () { + const PLUGIN_BASE_HOOKS = ` + + + + + + + + + `; + const PLUGIN_WINDOWS_HOOKS = ` + + + + + + + `; + const PLUGIN_ANDROID_HOOKS = ` + + + + + + + `; + + function addHooksToPlugin (hooksXml) { + const config = new PluginInfo(testPluginInstalledPath); + addHooks(hooksXml, config._et); + + const configPath = path.join(testPluginInstalledPath, 'plugin.xml'); + fs.writeFileSync(configPath, config._et.write({ indent: 4 })); + } + it('Test 009 : should execute hook scripts serially from plugin.xml', function () { - usePluginConfig('OnlyNonPlatformScripts'); + addHooksToPlugin(PLUGIN_BASE_HOOKS); return hooksRunner.fire(test_event, hookOptions) .then(checkHooksOrderFile); }); it('Test 010 : should execute hook scripts serially from plugin.xml including platform scripts', function () { - usePluginConfig('OnePlatform'); + addHooksToPlugin(PLUGIN_BASE_HOOKS); + addHooksToPlugin(PLUGIN_WINDOWS_HOOKS); return hooksRunner.fire(test_event, hookOptions) .then(checkHooksOrderFile); @@ -202,7 +273,9 @@ describe('HooksRunner', function () { globby.sync('scripts/**', { cwd: testPluginInstalledPath, absolute: true }) .forEach(f => fs.chmodSync(f, 0o755)); - usePluginConfig('TwoPlatforms'); + addHooksToPlugin(PLUGIN_BASE_HOOKS); + addHooksToPlugin(PLUGIN_WINDOWS_HOOKS); + addHooksToPlugin(PLUGIN_ANDROID_HOOKS); hookOptions.cordova.platforms = ['android']; return hooksRunner.fire(test_event, hookOptions).then(function () { diff --git a/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginOnePlatform_bat.xml b/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginOnePlatform_bat.xml deleted file mode 100644 index e1d6a0ca3..000000000 --- a/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginOnePlatform_bat.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - Plugin with hooks - - - - - - - - - - - - - \ No newline at end of file diff --git a/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginOnePlatform_sh.xml b/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginOnePlatform_sh.xml deleted file mode 100644 index fef675ad8..000000000 --- a/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginOnePlatform_sh.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - Plugin with hooks - - - - - - - - - - - - - \ No newline at end of file diff --git a/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginOnlyNonPlatformScripts_bat.xml b/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginOnlyNonPlatformScripts_bat.xml deleted file mode 100644 index fcbdb6199..000000000 --- a/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginOnlyNonPlatformScripts_bat.xml +++ /dev/null @@ -1,14 +0,0 @@ - - - Plugin with hooks - - - - - - - - \ No newline at end of file diff --git a/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginOnlyNonPlatformScripts_sh.xml b/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginOnlyNonPlatformScripts_sh.xml deleted file mode 100644 index 7593ffe68..000000000 --- a/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginOnlyNonPlatformScripts_sh.xml +++ /dev/null @@ -1,14 +0,0 @@ - - - Plugin with hooks - - - - - - - - \ No newline at end of file diff --git a/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginTwoPlatforms_bat.xml b/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginTwoPlatforms_bat.xml deleted file mode 100644 index 8fd6d38bc..000000000 --- a/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginTwoPlatforms_bat.xml +++ /dev/null @@ -1,24 +0,0 @@ - - - Plugin with hooks - - - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginTwoPlatforms_sh.xml b/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginTwoPlatforms_sh.xml deleted file mode 100644 index 6a4e11da8..000000000 --- a/spec/cordova/fixtures/plugins/com.plugin.withhooks/pluginTwoPlatforms_sh.xml +++ /dev/null @@ -1,24 +0,0 @@ - - - Plugin with hooks - - - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/spec/cordova/fixtures/projWithHooks/configOnePlatform_bat.xml b/spec/cordova/fixtures/projWithHooks/configOnePlatform_bat.xml deleted file mode 100644 index 6f1a691c5..000000000 --- a/spec/cordova/fixtures/projWithHooks/configOnePlatform_bat.xml +++ /dev/null @@ -1,22 +0,0 @@ - - - HelloCordova - - A sample Apache Cordova application that responds to the deviceready event. - - - Apache Cordova Team - - - - - - - - - - - - - - \ No newline at end of file diff --git a/spec/cordova/fixtures/projWithHooks/configOnePlatform_sh.xml b/spec/cordova/fixtures/projWithHooks/configOnePlatform_sh.xml deleted file mode 100644 index bddcd2563..000000000 --- a/spec/cordova/fixtures/projWithHooks/configOnePlatform_sh.xml +++ /dev/null @@ -1,22 +0,0 @@ - - - HelloCordova - - A sample Apache Cordova application that responds to the deviceready event. - - - Apache Cordova Team - - - - - - - - - - - - - - \ No newline at end of file diff --git a/spec/cordova/fixtures/projWithHooks/configOnlyNonPlatformScripts_bat.xml b/spec/cordova/fixtures/projWithHooks/configOnlyNonPlatformScripts_bat.xml deleted file mode 100644 index 8ad84f67a..000000000 --- a/spec/cordova/fixtures/projWithHooks/configOnlyNonPlatformScripts_bat.xml +++ /dev/null @@ -1,16 +0,0 @@ - - - HelloCordova - - A sample Apache Cordova application that responds to the deviceready event. - - - Apache Cordova Team - - - - - - - - \ No newline at end of file diff --git a/spec/cordova/fixtures/projWithHooks/configOnlyNonPlatformScripts_sh.xml b/spec/cordova/fixtures/projWithHooks/configOnlyNonPlatformScripts_sh.xml deleted file mode 100644 index 8634f64e4..000000000 --- a/spec/cordova/fixtures/projWithHooks/configOnlyNonPlatformScripts_sh.xml +++ /dev/null @@ -1,16 +0,0 @@ - - - HelloCordova - - A sample Apache Cordova application that responds to the deviceready event. - - - Apache Cordova Team - - - - - - - - \ No newline at end of file diff --git a/spec/cordova/fixtures/projWithHooks/configTwoPlatforms_bat.xml b/spec/cordova/fixtures/projWithHooks/configTwoPlatforms_bat.xml deleted file mode 100644 index 758ac7002..000000000 --- a/spec/cordova/fixtures/projWithHooks/configTwoPlatforms_bat.xml +++ /dev/null @@ -1,28 +0,0 @@ - - - HelloCordova - - A sample Apache Cordova application that responds to the deviceready event. - - - Apache Cordova Team - - - - - - - - - - - - - - - - - - - - \ No newline at end of file diff --git a/spec/cordova/fixtures/projWithHooks/configTwoPlatforms_sh.xml b/spec/cordova/fixtures/projWithHooks/configTwoPlatforms_sh.xml deleted file mode 100644 index 1175e6c5e..000000000 --- a/spec/cordova/fixtures/projWithHooks/configTwoPlatforms_sh.xml +++ /dev/null @@ -1,28 +0,0 @@ - - - HelloCordova - - A sample Apache Cordova application that responds to the deviceready event. - - - Apache Cordova Team - - - - - - - - - - - - - - - - - - - - \ No newline at end of file