Mailing-List: contact commits-help@cordova.apache.org; run by ezmlm
Precedence: bulk
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
From: steven@apache.org
To: commits@cordova.apache.org
Date: Thu, 22 Jun 2017 22:04:44 -0000
Message-Id: <1d9b817503bb4ee0bcbb26bcdef34267@git.apache.org>
In-Reply-To: <f31b879256f24fc991d92859f950c3cd@git.apache.org>
References: <f31b879256f24fc991d92859f950c3cd@git.apache.org>
Subject: [3/4] cordova-lib git commit: removed restoring platforms+plugins
 save.spec from prepare. rewrote structure of prepare unit tests,
 and included coverage for restoring platforms+plugins. slight changes to
 prepare.js so it leverages its own helper methods
archived-at: Thu, 22 Jun 2017 22:04:48 -0000

removed restoring platforms+plugins save.spec from prepare. rewrote structure of prepare unit tests, and included coverage for restoring platforms+plugins. slight changes to prepare.js so it leverages its own helper methods via its own export (helpful for testing).


Project: http://git-wip-us.apache.org/repos/asf/cordova-lib/repo
Commit: http://git-wip-us.apache.org/repos/asf/cordova-lib/commit/89d66935
Tree: http://git-wip-us.apache.org/repos/asf/cordova-lib/tree/89d66935
Diff: http://git-wip-us.apache.org/repos/asf/cordova-lib/diff/89d66935

Branch: refs/heads/master
Commit: 89d669359cf692b1ddd64e24612985ae86889306
Parents: 1a28929
Author: filmaj <maj.fil@gmail.com>
Authored: Mon Jun 19 16:04:25 2017 -0500
Committer: Steve Gill <stevengill97@gmail.com>
Committed: Thu Jun 22 13:55:17 2017 -0600

----------------------------------------------------------------------
 spec-cordova/prepare.spec.js | 229 ++++++++++++++------------------------
 src/cordova/prepare.js       |  67 +++++------
 2 files changed, 117 insertions(+), 179 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/89d66935/spec-cordova/prepare.spec.js
----------------------------------------------------------------------
diff --git a/spec-cordova/prepare.spec.js b/spec-cordova/prepare.spec.js
index 029841d..ba298a7 100644
--- a/spec-cordova/prepare.spec.js
+++ b/spec-cordova/prepare.spec.js
@@ -17,175 +17,110 @@
     under the License.
 */
 
-var shell = require('shelljs'),
-    PlatformJson = require('cordova-common').PlatformJson,
-    path = require('path'),
-    util = require('../src/cordova/util'),
-    prepare = require('../src/cordova/prepare'),
-    lazy_load = require('../src/cordova/lazy_load'),
-    ConfigParser = require('cordova-common').ConfigParser,
-    platforms = require('../src/platforms/platforms'),
-    HooksRunner = require('../src/hooks/HooksRunner'),
-    xmlHelpers = require('cordova-common').xmlHelpers,
-    et = require('elementtree'),
-    Q = require('q');
+/* eslint-env jasmine */
 
-var project_dir = '/some/path';
-var supported_platforms = Object.keys(platforms).filter(function(p) { return p != 'www'; });
-
-var TEST_XML = '<?xml version="1.0" encoding="UTF-8"?>\n' +
-    '<widget xmlns     = "http://www.w3.org/ns/widgets"\n' +
-    '        xmlns:cdv = "http://cordova.apache.org/ns/1.0"\n' +
-    '        id        = "io.cordova.hellocordova"\n' +
-    '        version   = "0.0.1">\n' +
-    '    <name>Hello Cordova</name>\n' +
-    '    <description>\n' +
-    '        A sample Apache Cordova application that responds to the deviceready event.\n' +
-    '    </description>\n' +
-    '    <author href="http://cordova.io" email="dev@cordova.apache.org">\n' +
-    '        Apache Cordova Team\n' +
-    '    </author>\n' +
-    '    <content src="index.html" />\n' +
-    '    <access origin="*" />\n' +
-    '    <preference name="fullscreen" value="true" />\n' +
-    '    <preference name="webviewbounce" value="true" />\n' +
-    '</widget>\n';
+var path = require('path');
+var rewire = require('rewire');
+var util = require('../src/cordova/util');
+var cordova_config = require('../src/cordova/config');
+var prepare = rewire('../src/cordova/prepare');
+var restore = require('../src/cordova/restore-util');
+var platforms = require('../src/platforms/platforms');
+var HooksRunner = require('../src/hooks/HooksRunner');
+var Q = require('q');
+var PlatformJson = require('cordova-common').PlatformJson;
 
-describe('prepare command', function() {
-    var is_cordova,
-        cd_project_root,
-        list_platforms,
-        fire,
-        parsers = {},
-        find_plugins,
-        cp,
-        mkdir,
-        load, platformApi, getPlatformApi;
+var project_dir = '/some/path';
 
+describe('cordova/prepare', function () {
+    var platform_api_prepare_mock;
     beforeEach(function () {
-        getPlatformApi = spyOn(platforms, 'getPlatformApi').and.callFake(function (platform, rootDir) {
+        platform_api_prepare_mock = jasmine.createSpy('prepare').and.returnValue(Q());
+        spyOn(platforms, 'getPlatformApi').and.callFake(function (platform, rootDir) {
             return {
-                prepare: jasmine.createSpy('prepare').and.returnValue(Q()),
+                prepare: platform_api_prepare_mock,
                 getPlatformInfo: jasmine.createSpy('getPlatformInfo').and.returnValue({
                     locations: {
                         www: path.join(project_dir, 'platforms', platform, 'www')
                     }
-                }),
+                })
             };
         });
-        is_cordova = spyOn(util, 'isCordova').and.returnValue(project_dir);
-        cd_project_root = spyOn(util, 'cdProjectRoot').and.returnValue(project_dir);
-        list_platforms = spyOn(util, 'listPlatforms').and.returnValue(supported_platforms);
-        fire = spyOn(HooksRunner.prototype, 'fire').and.returnValue(Q());
-
-        find_plugins = spyOn(util, 'findPlugins').and.returnValue([]);
-        spyOn(PlatformJson, 'load').and.returnValue(new PlatformJson(null, null, {}));
-        spyOn(PlatformJson.prototype, 'save');
-        load = spyOn(lazy_load, 'based_on_config').and.returnValue(Q());
-        cp = spyOn(shell, 'cp').and.returnValue(true);
-        mkdir = spyOn(shell, 'mkdir');
-        spyOn(ConfigParser.prototype, 'write');
-        spyOn(xmlHelpers, 'parseElementtreeSync').and.callFake(function() {
-            return new et.ElementTree(et.XML(TEST_XML));
-        });
+        spyOn(PlatformJson, 'load');
     });
 
-    describe('failure', function() {
-        it('Test 001 : should not run outside of a cordova-based project by calling util.isCordova', function(done) {
-            is_cordova.and.returnValue(false);
-            cd_project_root.and.callThrough();  // undo spy here because prepare depends on cdprojectRoot for isCordova check
-            prepare().then(function() {
-                expect('this call').toBe('fail');
-            }, function(err) {
-                expect('' + err).toContain('Current working directory is not a Cordova-based project.');
-            }).fin(done);
+    describe('main method', function () {
+        beforeEach(function () {
+            spyOn(util, 'cdProjectRoot').and.returnValue(project_dir);
+            spyOn(util, 'preProcessOptions').and.returnValue({
+                platforms: []
+            });
+            spyOn(cordova_config, 'read').and.returnValue({});
+            spyOn(HooksRunner.prototype, 'fire').and.returnValue(Q());
+            spyOn(restore, 'installPlatformsFromConfigXML').and.returnValue(Q());
+            spyOn(restore, 'installPluginsFromConfigXML').and.returnValue(Q());
         });
-        it('Test 002 : should not run inside a cordova-based project with no platforms', function(done) {
-            list_platforms.and.returnValue([]);
-            prepare().then(function() {
-                expect('this call').toBe('fail');
-            }, function(err) {
-                expect('' + err).toContain('No platforms added to this project. Please use `cordova platform add <platform>`.');
-            }).fin(done);
+        describe('failure', function () {
+            it('should invoke util.preProcessOptions as preflight task checker, which, if fails, should trigger promise rejection');
+            it('should not fire any hooks if preProcessOptions throws');
         });
-    });
 
-    describe('success', function() {
-        it('Test 003 : should run inside a Cordova-based project by calling util.isCordova', function(done) {
-            prepare().then(function() {
-                expect(is_cordova).toHaveBeenCalled();
-            }, function(err) {
-                expect(err).toBeUndefined();
-            }).fin(done);
-        });
-        it('Test 004 : should get PlatformApi instance for each platform and invoke its\' run method', function(done) {
-            prepare().then(function() {
-                supported_platforms.forEach(function(p) {
-                    expect(parsers[p]).toHaveBeenCalled();
-                    expect(getPlatformApi).toHaveBeenCalledWith(p);
-                });
-                expect(platformApi.run).toHaveBeenCalled();
-            }, function(err) {
-                expect(err).toBeUndefined();
-            }).fin(done);
+        describe('success', function () {
+            it('should fire the before_prepare hook and provide platform and path information as arguments');
+            it('should invoke restore module\'s installPlatformsFromConfigXML method');
+            it('should retrieve PlatformApi instances for each platform provided');
+            it('should invoke restore module\'s installPluginsFromConfigXML method');
+            it('should invoke preparePlatforms method, providing the appropriate platforms');
+            it('should fire the after_prepare hook and provide platform and path information as arguments');
         });
     });
 
-    describe('hooks', function() {
-        describe('when platforms are added', function() {
-            it('Test 005 : should fire before hooks through the hooker module, and pass in platforms and paths as data object', function(done) {
-                prepare().then(function() {
-                    expect(fire.calls.argsFor(0)).toEqual(
-                    [ 'before_prepare',
-                    { verbose: false,
-                        platforms:
-                         [ 'ios',
-                           'osx',
-                           'android',
-                           'ubuntu',
-                           'blackberry10',
-                           'windows',
-                           'webos',
-                           'browser' ],
-                        options: {},
-                        save: false,
-                        fetch: false,
-                        paths:
-                         [ path.join('/','some','path','platforms','ios','www'),
-                           path.join('/','some','path','platforms','osx','www'),
-                           path.join('/','some','path','platforms','android','www'),
-                           path.join('/','some','path','platforms','ubuntu','www'),
-                           path.join('/','some','path','platforms','blackberry10','www'),
-                           path.join('/','some','path','platforms','windows','www'),
-                           path.join('/','some','path','platforms','webos','www'),
-                           path.join('/','some','path','platforms','browser','www') ],
-                        searchpath: undefined } ]);
-                }, function(err) {
-                    expect(err).toBeUndefined();
-                }).fin(done);
-            });
-            it('Test 006 : should fire after hooks through the hooker module, and pass in platforms and paths as data object', function(done) {
-                prepare('android').then(function() {
-                    expect(fire.calls.argsFor(1)).toEqual([ 'after_prepare',{ platforms: [ 'android' ],verbose: false,options: {},paths: [ path.join( '/','some','path', 'platforms', 'android', 'www' ) ],searchpath: undefined } ]);
-                }, function(err) {
-                    expect(err).toBeUndefined('Exception while running `prepare android`:\n' + err.stack);
-                }).fin(done);
+    describe('preparePlatforms helper method', function () {
+        var cfg_parser_mock = function () {};
+        var cfg_parser_revert_mock;
+        var platform_munger_mock = function () {};
+        var platform_munger_revert_mock;
+        var platform_munger_save_mock;
+        beforeEach(function () {
+            cfg_parser_mock.prototype = jasmine.createSpyObj('config parser prototype mock', []);
+            cfg_parser_revert_mock = prepare.__set__('ConfigParser', cfg_parser_mock);
+            platform_munger_save_mock = jasmine.createSpy('platform munger save mock');
+            platform_munger_mock.protytpe = jasmine.createSpyObj('platform munger prototype mock', ['add_config_changes']);
+            platform_munger_mock.prototype.add_config_changes.and.returnValue({
+                save_all: platform_munger_save_mock
             });
+            platform_munger_revert_mock = prepare.__set__('PlatformMunger', platform_munger_mock);
+            spyOn(util, 'projectConfig').and.returnValue(project_dir);
+            spyOn(util, 'projectWww').and.returnValue(path.join(project_dir, 'www'));
+            spyOn(prepare, 'restoreMissingPluginsForPlatform').and.returnValue(Q());
         });
+        afterEach(function () {
+            cfg_parser_revert_mock();
+            platform_munger_revert_mock();
+        });
+        it('should call restoreMissingPluginsForPlatform');
+        it('should retrieve the platform API via getPlatformApi per platform provided, and invoke the prepare method from that API');
+        it('should fire a pre_package hook for the windows platform when the platform API is not an instance of PlatformApiPoly');
+        // TODO: xit'ed the one below as dynamic requires make it difficult to spy on
+        // Can we refactor the relevant code to make it testable?
+        xit('should invoke browserify if the browserify option is provided');
+        it('should handle config changes by invoking add_config_changes and save_all');
+    });
 
-        describe('with no platforms added', function() {
-            beforeEach(function() {
-                list_platforms.and.returnValue([]);
-            });
-            it('Test 007 : should not fire the hooker', function(done) {
-                Q().then(prepare).then(function() {
-                    expect('this call').toBe('fail');
-                }, function(err) {
-                    expect(err).toEqual(jasmine.any(Error));
-                    expect(fire).not.toHaveBeenCalledWith('before_prepare');
-                    expect(fire).not.toHaveBeenCalledWith('after_prepare');
-                }).fin(done);
+    describe('restoreMissingPluginsForPlatform helper method', function () {
+        var is_plugin_installed_mock;
+        beforeEach(function () {
+            is_plugin_installed_mock = jasmine.createSpy('is plugin installed mock');
+            // mock platform json value below
+            PlatformJson.load.and.returnValue({
+                isPluginInstalled: is_plugin_installed_mock,
+                root: {
+                    installed_plugins: [],
+                    dependent_plugins: []
+                }
             });
         });
+        it('should resolve quickly if there is no difference between "old" and "new" platform.json');
+        it('should leverage platform API to remove and add any missing plugins identified');
     });
-});
\ No newline at end of file
+});

http://git-wip-us.apache.org/repos/asf/cordova-lib/blob/89d66935/src/cordova/prepare.js
----------------------------------------------------------------------
diff --git a/src/cordova/prepare.js b/src/cordova/prepare.js
index 729813d..2f83fe2 100644
--- a/src/cordova/prepare.js
+++ b/src/cordova/prepare.js
@@ -17,25 +17,27 @@
     under the License.
 */
 
-var cordova_util      = require('./util'),
-    ConfigParser      = require('cordova-common').ConfigParser,
-    PlatformJson      = require('cordova-common').PlatformJson,
-    PluginInfoProvider = require('cordova-common').PluginInfoProvider,
-    PlatformMunger    = require('cordova-common').ConfigChanges.PlatformMunger,
-    events            = require('cordova-common').events,
-    platforms         = require('../platforms/platforms'),
-    PlatformApiPoly = require('../platforms/PlatformApiPoly'),
-    HooksRunner       = require('../hooks/HooksRunner'),
-    Q                 = require('q'),
-    restore           = require('./restore-util'),
-    path              = require('path'),
-    config            = require('./config'),
-    _ = require('underscore');
+var cordova_util = require('./util');
+var ConfigParser = require('cordova-common').ConfigParser;
+var PlatformJson = require('cordova-common').PlatformJson;
+var PluginInfoProvider = require('cordova-common').PluginInfoProvider;
+var PlatformMunger = require('cordova-common').ConfigChanges.PlatformMunger;
+var events = require('cordova-common').events;
+var platforms = require('../platforms/platforms');
+var PlatformApiPoly = require('../platforms/PlatformApiPoly');
+var HooksRunner = require('../hooks/HooksRunner');
+var Q = require('q');
+var restore = require('./restore-util');
+var path = require('path');
+var config = require('./config');
+var _ = require('underscore');
 
-// Returns a promise.
 exports = module.exports = prepare;
-function prepare(options) {
-    return Q().then(function() {
+module.exports.preparePlatforms = preparePlatforms;
+module.exports.restoreMissingPluginsForPlatform = restoreMissingPluginsForPlatform;
+
+function prepare (options) {
+    return Q().then(function () {
         var projectRoot = cordova_util.cdProjectRoot();
         var config_json = config.read(projectRoot);
         options = options || { verbose: false, platforms: [], options: {} };
@@ -43,12 +45,12 @@ function prepare(options) {
         options.fetch = options.fetch || false;
         var hooksRunner = new HooksRunner(projectRoot);
         return hooksRunner.fire('before_prepare', options)
-        .then(function(){
-            return restore.installPlatformsFromConfigXML(options.platforms, { searchpath : options.searchpath, fetch : options.fetch, restoring : true });
+        .then(function () {
+            return restore.installPlatformsFromConfigXML(options.platforms, { searchpath: options.searchpath, fetch: options.fetch, restoring: true });
         })
-        .then(function(){
+        .then(function () {
             options = cordova_util.preProcessOptions(options);
-            var paths = options.platforms.map(function(p) {
+            var paths = options.platforms.map(function (p) {
                 var platform_path = path.join(projectRoot, 'platforms', p);
                 return platforms.getPlatformApi(p, platform_path).getPlatformInfo().locations.www;
             });
@@ -56,13 +58,13 @@ function prepare(options) {
         }).then(function () {
             options = cordova_util.preProcessOptions(options);
             return restore.installPluginsFromConfigXML(options);
-        }).then(function() {
+        }).then(function () {
             options = cordova_util.preProcessOptions(options);
             options.searchpath = options.searchpath || config_json.plugin_search_path;
             // Iterate over each added platform
-            return preparePlatforms(options.platforms, projectRoot, options);
-        }).then(function() {
-            options.paths = options.platforms.map(function(platform) {
+            return module.exports.preparePlatforms(options.platforms, projectRoot, options);
+        }).then(function () {
+            options.paths = options.platforms.map(function (platform) {
                 return platforms.getPlatformApi(platform).getPlatformInfo().locations.www;
             });
             return hooksRunner.fire('after_prepare', options);
@@ -79,7 +81,7 @@ function prepare(options) {
  * @return  {Promise}
  */
 function preparePlatforms (platformList, projectRoot, options) {
-    return Q.all(platformList.map(function(platform) {
+    return Q.all(platformList.map(function (platform) {
         // TODO: this need to be replaced by real projectInfo
         // instance for current project.
         var project = {
@@ -92,7 +94,7 @@ function preparePlatforms (platformList, projectRoot, options) {
         };
 
         // CB-9987 We need to reinstall the plugins for the platform it they were added by cordova@<5.4.0
-        return restoreMissingPluginsForPlatform(platform, projectRoot, options)
+        return module.exports.restoreMissingPluginsForPlatform(platform, projectRoot, options)
         .then(function () {
             // platformApi prepare takes care of all functionality
             // which previously had been executed by cordova.prepare:
@@ -116,6 +118,7 @@ function preparePlatforms (platformList, projectRoot, options) {
             })
             .then(function () {
                 if (options.browserify) {
+                    // TODO: dynamic require here makes it difficult to test this code branch.
                     var browserify = require('../plugman/browserify');
                     return browserify(project, platformApi);
                 }
@@ -125,14 +128,13 @@ function preparePlatforms (platformList, projectRoot, options) {
                 var platformRoot = path.join(projectRoot, 'platforms', platform);
                 var platformJson = PlatformJson.load(platformRoot, platform);
                 var munger = new PlatformMunger(platform, platformRoot, platformJson);
-                munger.add_config_changes(project.projectConfig, /*should_increment=*/true).save_all();
+                // the boolean argument below is "should_increment"
+                munger.add_config_changes(project.projectConfig, true).save_all();
             });
         });
     }));
 }
 
-module.exports.preparePlatforms = preparePlatforms;
-
 /**
  * Ensures that plugins, installed with previous versions of CLI (<5.4.0) are
  *   readded to platform correctly. Also triggers regeneration of
@@ -148,7 +150,7 @@ module.exports.preparePlatforms = preparePlatforms;
  * @return  {Promise}               Promise that'll be fulfilled if all the
  *   plugins reinstalled properly.
  */
-function restoreMissingPluginsForPlatform(platform, projectRoot, options) {
+function restoreMissingPluginsForPlatform (platform, projectRoot, options) {
     events.emit('verbose', 'Checking for any plugins added to the project that have not been installed in ' + platform + ' platform');
 
     // Flow:
@@ -162,11 +164,12 @@ function restoreMissingPluginsForPlatform(platform, projectRoot, options) {
     var missingPlugins = Object.keys(oldPlatformJson.root.installed_plugins)
         .concat(Object.keys(oldPlatformJson.root.dependent_plugins))
         .reduce(function (result, candidate) {
-            if (!platformJson.isPluginInstalled(candidate))
+            if (!platformJson.isPluginInstalled(candidate)) {
                 result.push({name: candidate,
                     // Note: isPluginInstalled is actually returns not a boolean,
                     // but object which corresponds to this particular plugin
                     variables: oldPlatformJson.isPluginInstalled(candidate)});
+            }
 
             return result;
         }, []);


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org