cordova-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CB-12361) Speed up cordova-lib tests
Date Wed, 21 Jun 2017 20:40:03 GMT

    [ https://issues.apache.org/jira/browse/CB-12361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16058183#comment-16058183
] 

ASF GitHub Bot commented on CB-12361:
-------------------------------------

Github user filmaj commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/567#discussion_r123359362
  
    --- Diff: spec-cordova/prepare.spec.js ---
    @@ -17,175 +17,265 @@
         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;
    +var PluginInfoProvider = require('cordova-common').PluginInfoProvider;
     
    -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 () {
    +    let platform_api_prepare_mock;
    +    let platform_api_add_mock;
    +    let platform_api_remove_mock;
         beforeEach(function () {
    -        getPlatformApi = spyOn(platforms, 'getPlatformApi').and.callFake(function (platform,
rootDir) {
    +        platform_api_prepare_mock = jasmine.createSpy('prepare').and.returnValue(Q());
    +        platform_api_add_mock = jasmine.createSpy('add').and.returnValue(Q());
    +        platform_api_remove_mock = jasmine.createSpy('remove').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')
                         }
                     }),
    +                removePlugin: platform_api_add_mock,
    +                addPlugin: platform_api_remove_mock
                 };
             });
    -        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');
    +        spyOn(HooksRunner.prototype, 'fire').and.returnValue(Q());
    +        spyOn(util, 'isCordova').and.returnValue(true);
         });
     
    -    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(cordova_config, 'read').and.returnValue({});
    +            spyOn(restore, 'installPlatformsFromConfigXML').and.returnValue(Q());
    +            spyOn(restore, 'installPluginsFromConfigXML').and.returnValue(Q());
    +            
    +            spyOn(prepare, 'preparePlatforms').and.returnValue(Q);
    +        });
    +        describe('failure', function () {
    +            it('should invoke util.preProcessOptions as preflight task checker, which,
if fails, should trigger promise rejection and only fire the before_prepare hook', function(done)
{
    +                spyOn(util, 'cdProjectRoot').and.returnValue(project_dir);
    +                spyOn(util, 'preProcessOptions').and.throwError();
    +                prepare({}).fail(function(err) {
    +                    expect(err).toBeDefined();
    +                    expect(HooksRunner.prototype.fire.calls.count()).toBe(1);
    +                    expect(HooksRunner.prototype.fire.calls.argsFor(0)[0]).toEqual('before_prepare');
    +                    done();
    +                });
    +            });
    +            it('should invoke util.cdProjectRoot as a preflight task checker, which,
if fails, should trigger a promise rejection and fire no hooks', function(done) {
    +                spyOn(util, 'cdProjectRoot').and.throwError();
    +                prepare({}).fail(function(err) {
    +                    expect(err).toBeDefined();
    +                    expect(HooksRunner.prototype.fire.calls.any()).toBe(false);
    +                    done();
    +                });
    +            });
             });
    -        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('success', function () {
    +            beforeEach(function () {
    +                spyOn(util, 'cdProjectRoot').and.returnValue(project_dir);
    +                spyOn(util, 'preProcessOptions').and.callFake(function(options) {
    +                    let platforms = options.platforms || [];
    +                    return {'platforms':platforms};
    +                });
    +            });
    +            it('should fire the before_prepare hook and provide platform and path information
as arguments', function(done) {
    +                prepare({}).then(function() {
    +                    expect(HooksRunner.prototype.fire.calls.argsFor(0)[0]).toEqual('before_prepare');
    +                    done();
    +                });
    +            });
    +            it('should invoke restore module\'s installPlatformsFromConfigXML method',
function(done) {
    +                prepare({}).then(function() {
    +                    expect(restore.installPlatformsFromConfigXML).toHaveBeenCalled();
    +                    done();
    +                }).fail(function(err){
    +                    expect(err).toBeUndefined();
    +                    done();
    +                });
    +            });
    +            it('should retrieve PlatformApi instances for each platform provided', function(done)
{
    +                prepare({'platforms':['android', 'ios']}).then(function() {
    +                    expect(platforms.getPlatformApi.calls.count()).toBe(4);
    +                    expect(platforms.getPlatformApi.calls.argsFor(0)[0]).toBe('android');
    +                    expect(platforms.getPlatformApi.calls.argsFor(0)[1]).toBe('/some/path/platforms/android');
    +                    expect(platforms.getPlatformApi.calls.argsFor(1)[0]).toBe('ios');
    +                    expect(platforms.getPlatformApi.calls.argsFor(1)[1]).toBe('/some/path/platforms/ios');
    +                    expect(platforms.getPlatformApi.calls.argsFor(2)[0]).toBe('android');
    +                    expect(platforms.getPlatformApi.calls.argsFor(3)[0]).toBe('ios');
    +                    done();
    +                }).fail(function(err){
    +                    expect(err).toBeUndefined();
    +                    done();
    +                });
    +            });
    +            it('should invoke restore module\'s installPluginsFromConfigXML method',
function(done) {
    +                prepare({platforms:[]}).then(function() {
    +                    expect(restore.installPluginsFromConfigXML).toHaveBeenCalled();
    +                    done();
    +                });
    +            });
    +            it('should invoke preparePlatforms method, providing the appropriate platforms',
function(done) {
    +                prepare({platforms:['android']}).then(function() {
    +                    expect(prepare.preparePlatforms).toHaveBeenCalled();
    +                    expect(prepare.preparePlatforms.calls.argsFor(0)[0][0]).toBe('android');
    +                    expect(prepare.preparePlatforms.calls.argsFor(0)[1]).toBe('/some/path');
    +                    done();
    +                });
    +            });
    +            it('should fire the after_prepare hook and provide platform and path information
as arguments', function(done) {
    +                prepare({platforms:['android']}).then(function() {
    +                    expect(HooksRunner.prototype.fire.calls.argsFor(1)[0]).toEqual('after_prepare');
    +                    expect(HooksRunner.prototype.fire.calls.argsFor(1)[1]).toEqual({
'platforms': [ 'android' ], 'paths': [ '/some/path/platforms/android/www' ], 'searchpath':
undefined });
    +                    expect(HooksRunner.prototype.fire.calls.count()).toBe(2);
    +                    done();
    +                });
    +            });
             });
         });
     
    -    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) {
    +    describe('preparePlatforms helper method', function () {
    +        let cfg_parser_mock = function () {};
    +        let cfg_parser_revert_mock;
    +        let platform_munger_mock = function () {};
    +        let platform_munger_revert_mock;
    +        let platform_munger_save_mock;
    +        beforeEach(function () {
    +            spyOn(prepare, 'restoreMissingPluginsForPlatform').and.returnValue(Q());
    +            //cfg_parser_mock.prototype = jasmine.createSpyObj('config parser prototype
mock', ['blah']);
    +            //cfg_parser_mock.prototype = {};
    +            cfg_parser_revert_mock = prepare.__set__('ConfigParser', cfg_parser_mock);
    +            platform_munger_save_mock = jasmine.createSpy('platform munger save mock');
    +            platform_munger_mock.prototype = 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'));
    +            
    +        });
    +        afterEach(function () {
    +            cfg_parser_revert_mock();
    +            platform_munger_revert_mock();
    +        });
    +        it('should call restoreMissingPluginsForPlatform', function(done) {
    +            prepare.preparePlatforms(['android'],project_dir, {}).then(function() {
    +                expect(prepare.restoreMissingPluginsForPlatform).toHaveBeenCalled();
    +                done();
    +            }).fail(function(err) {
                     expect(err).toBeUndefined();
    -            }).fin(done);
    +                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) {
    +        it('should retrieve the platform API via getPlatformApi per platform provided,
and invoke the prepare method from that API', function(done) {
    +            prepare.preparePlatforms(['android'],project_dir, {}).then(function() {
    +                expect(platforms.getPlatformApi.calls.count()).toBe(1);
    +                expect(platforms.getPlatformApi.calls.argsFor(0)[0]).toBe('android');
    +                done();
    +            }).fail(function(err) {
    +                expect(err).toBeUndefined();
    +                done();
    +            });
    +        });
    +        it('should fire a pre_package hook for the windows', function(done) {
    +            prepare.preparePlatforms(['windows'],project_dir, {}).then(function() {
    +                expect(HooksRunner.prototype.fire.calls.count()).toBe(1);
    +                expect(HooksRunner.prototype.fire.calls.argsFor(0)[0]).toBe('pre_package');
    +                done();
    +            }).fail(function(err) {
    +                expect(err).toBeUndefined();
    +                done();
    +            });
    +        });
    +        // 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',
function(done) {
    +            prepare.preparePlatforms(['android'],project_dir, {}).then(function() {
    +                expect(platform_munger_mock.prototype.add_config_changes).toHaveBeenCalled();
    +                expect(platform_munger_save_mock).toHaveBeenCalled();
    +                done();
    +            }).fail(function(err) {
                     expect(err).toBeUndefined();
    -            }).fin(done);
    +                done();
    +            });
             });
         });
     
    -    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);
    +    describe('restoreMissingPluginsForPlatform helper method', function () {
    +        let is_plugin_installed_mock;
    +        let is_plugin_provider_get_mock;
    +        it('should resolve quickly if there is no difference between "old" and "new"
platform.json', function(done) {
    +            is_plugin_installed_mock = jasmine.createSpy('is plugin installed mock');
    +            // mock platform json value below
    +            PlatformJson.load.and.callFake(function(platformJsonPath, plat) {
    +                return {
    +                    isPluginInstalled: is_plugin_installed_mock,
    +                    root: {
    +                        installed_plugins: [],
    +                        dependent_plugins: []
    +                    }
    +                };
                 });
    -            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);
    +
    +            prepare.restoreMissingPluginsForPlatform('android', project_dir, {}).then(function()
{
    +                expect(platforms.getPlatformApi).not.toHaveBeenCalled();
    +                done();
    +            }).fail(function(err) {
    +                expect(err).toBeUndefined();
    +                done();
                 });
             });
    -
    -        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);
    +        it('should leverage platform API to remove and add any missing plugins identified',
function(done) {
    +            is_plugin_installed_mock = jasmine.createSpy('is plugin installed mock');
    +            is_plugin_provider_get_mock = jasmine.createSpy('is plugin provider get mock');
    +            // mock platform json value below
    +            PlatformJson.load.and.callFake(function(platformJsonPath, plat) {
    +                //set different installed plugins to simulate missing plugins
    +                let missingPlugins;
    +                if (platformJsonPath === '/some/path/platforms/android') {
    +                    missingPlugins = {'cordova-plugin-device': {}};
    +                } else {
    +                    missingPlugins = {'cordova-plugin-camera': {}};
    +                }
    +                return {
    +                    isPluginInstalled: is_plugin_installed_mock,
    +                    root: {
    +                        installed_plugins: missingPlugins,
    +                        dependent_plugins: []
    +                    }
    +                };
    +            });
    +            spyOn(PluginInfoProvider.prototype, 'get').and.callFake(function() {
    +                return is_plugin_provider_get_mock;
    +            });
    +            prepare.restoreMissingPluginsForPlatform('android', project_dir, {}).then(function()
{
    +                expect(platforms.getPlatformApi).toHaveBeenCalled();
    +                expect(platform_api_add_mock).toHaveBeenCalled();
    +                expect(platform_api_remove_mock).toHaveBeenCalled();
    +                expect(PluginInfoProvider.prototype.get).toHaveBeenCalled();
    +                done();
    +            }).fail(function(err) {
    +                expect(err).toBeUndefined();
    +                done();
    --- End diff --
    
    `done(done);` please


> Speed up cordova-lib tests
> --------------------------
>
>                 Key: CB-12361
>                 URL: https://issues.apache.org/jira/browse/CB-12361
>             Project: Apache Cordova
>          Issue Type: Improvement
>          Components: cordova-lib
>            Reporter: Steve Gill
>            Assignee: Steve Gill
>              Labels: cordova-next
>
> * Split out e2e tests into own folder
> * stub i/o and network requests
> * use local fixtures when possible & makes sense



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

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


Mime
View raw message