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 Thu, 29 Jun 2017 19:41:01 GMT

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

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

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

    https://github.com/apache/cordova-lib/pull/573#discussion_r124878766
  
    --- Diff: spec/cordova/platform/addHelper.spec.js ---
    @@ -16,34 +16,459 @@
     */
     /* eslint-env jasmine */
     
    +var path = require('path');
    +var fs = require('fs');
    +var Q = require('q');
    +var shell = require('shelljs');
    +var events = require('cordova-common').events;
    +var rewire = require('rewire');
    +var platform_addHelper = rewire('../../../src/cordova/platform/addHelper');
    +var platform_module = require('../../../src/cordova/platform');
    +var platform_metadata = require('../../../src/cordova/platform_metadata');
    +var cordova_util = require('../../../src/cordova/util');
    +var cordova_config = require('../../../src/cordova/config');
    +var plugman = require('../../../src/plugman/plugman');
    +var fetch_metadata = require('../../../src/plugman/util/metadata');
    +var lazy_load = require('../../../src/cordova/lazy_load');
    +// require module here
    +// spy on it and return 
    +var cordova = require('../../../src/cordova/cordova');
    +var prepare = require('../../../src/cordova/prepare');
    +var gitclone = require('../../../src/gitclone');
    +var fail;
    +
     describe('cordova/platform/addHelper', function () {
    +    var projectRoot = '/some/path';
    +    // These _mock and _revert_mock objects use rewire as the modules these mocks replace
    +    // during testing all return functions, which we cannot spy on using jasmine.
    +    // Thus, we replace these modules inside the scope of addHelper.js using rewire,
and shim
    +    // in these _mock test dummies. The test dummies themselves are constructed using
    +    // jasmine.createSpy inside the first beforeEach.
    +    var cfg_parser_mock = function () {};
    +    var cfg_parser_revert_mock;
    +    var hooks_mock;
    +    var platform_api_mock;
    +    var fetch_mock;
    +    var fetch_revert_mock;
    +    var prepare_mock;
    +    var prepare_revert_mock;
    +    var fake_platform = {
    +        'platform': 'atari'
    +    };
    +    beforeEach(function () {
    +        hooks_mock = jasmine.createSpyObj('hooksRunner mock', ['fire']);
    +        hooks_mock.fire.and.returnValue(Q());
    +        cfg_parser_mock.prototype = jasmine.createSpyObj('config parser mock', ['write',
'removeEngine', 'addEngine', 'getHookScripts']);
    +        cfg_parser_revert_mock = platform_addHelper.__set__('ConfigParser', cfg_parser_mock);
    +        fetch_mock = jasmine.createSpy('fetch mock').and.returnValue(Q());
    +        fetch_revert_mock = platform_addHelper.__set__('fetch', fetch_mock);
    +        prepare_mock = jasmine.createSpy('prepare mock').and.returnValue(Q());
    +        prepare_mock.preparePlatforms = jasmine.createSpy('preparePlatforms mock').and.returnValue(Q());
    +        prepare_revert_mock = platform_addHelper.__set__('prepare', prepare_mock);
    +        spyOn(shell, 'mkdir');
    +        spyOn(fs, 'existsSync').and.returnValue(false);
    +        spyOn(fs, 'writeFileSync');
    +        spyOn(cordova_util, 'projectConfig').and.returnValue(path.join(projectRoot, 'config.xml'));
    +        spyOn(cordova_util, 'isDirectory').and.returnValue(false);
    +        spyOn(cordova_util, 'fixRelativePath').and.callFake(function (input) { return
input; });
    +        spyOn(cordova_util, 'isUrl').and.returnValue(false);
    +        spyOn(cordova_util, 'hostSupports').and.returnValue(true);
    +        spyOn(cordova_util, 'removePlatformPluginsJson');
    +        spyOn(cordova_config, 'read').and.returnValue({});
    +        // Fake platform details we will use for our mocks, returned by either
    +        // getPlatfromDetailsFromDir (in the local-directory case), or
    +        // downloadPlatform (in every other case)
    +        spyOn(platform_module, 'getPlatformDetailsFromDir').and.returnValue(Q(fake_platform));
    +        spyOn(platform_addHelper, 'downloadPlatform').and.returnValue(Q(fake_platform));
    +        spyOn(platform_addHelper, 'getVersionFromConfigFile').and.returnValue(false);
    +        spyOn(platform_addHelper, 'installPluginsForNewPlatform').and.returnValue(Q());
    +        platform_api_mock = jasmine.createSpyObj('platform api mock', ['createPlatform',
'updatePlatform']);
    +        platform_api_mock.createPlatform.and.returnValue(Q());
    +        platform_api_mock.updatePlatform.and.returnValue(Q());
    +        spyOn(cordova_util, 'getPlatformApiFunction').and.returnValue(platform_api_mock);
    +        spyOn(platform_metadata, 'save');
    +    });
    +    afterEach(function () {
    +        cfg_parser_revert_mock();
    +        fetch_revert_mock();
    +        prepare_revert_mock();
    +    });
         describe('error/warning conditions', function () {
    -        it('should require specifying at least one platform');
    -        it('should warn if host OS does not support the specified platform');
    +        it('should require specifying at least one platform', function (done) {
    +            platform_addHelper('add', hooks_mock).then(function () {
    +                fail('addHelper success handler unexpectedly invoked');
    +            }).fail(function (e) {
    +                expect(e.message).toContain('No platform specified.');
    +            }).done(done);
    +        });
    +
    +        it('should log if host OS does not support the specified platform', function
() {
    +            cordova_util.hostSupports.and.returnValue(false);
    +            spyOn(events, 'emit');
    +            platform_addHelper('add', hooks_mock, projectRoot, ['atari']);
    +            expect(events.emit.calls.mostRecent().args[1]).toContain('can not be built
on this OS');
    +        });
    +
    +        it('should throw if platform was already added before adding', function (done)
{
    +            fs.existsSync.and.returnValue('/some/path/platforms/ios');
    +            spyOn(cordova_util, 'requireNoCache').and.returnValue(true);
    +            platform_addHelper('add', hooks_mock, projectRoot, ['ios']).then(function
() {
    +                fail('addHelper should throw error');
    +            }).fail(function (e) {
    +                expect(e.message).toContain('already added.');
    +            }).done(done);
    +        });
    +
    +        it('should throw if platform was not added before updating', function(done) {
    +            platform_addHelper('update', hooks_mock, projectRoot, ['atari']).then(function
() {
    +                fail('addHelper should throw error');
    +            }).fail(function (e) {
    +                expect(e.message).toContain('Platform "atari" is not yet added. See `cordova
platform list`.');
    +            }).done(done);
    +        });
         });
         describe('happy path (success conditions)', function () {
    -        it('should fire the before_platform_* hook');
    -        it('should warn about deprecated platforms');
    -        /*
    -         * first "leg" (`then`) of the promise is platform "spec" support:
    -         * - tries to infer spec from either package.json or config.xml
    -         * - defaults to pinned version for platform.
    -         * - downloads the platform, passing in spec.
    -         * --> how to test: spy on downloadPlatform, validate spec passed.
    -         * --> should mock out downloadPlatform completely for all happy path tests.
this would short-circuit the first addHelper promise `then`.
    -         * second "leg" (`then`) of the promise:
    -         * - checks for already-added or not-added platform requirements. TODO: couldnt
we move this up to before downloading, to the initial error/warning checks?
    -         * - invokes platform api createPlatform or updatePlatform
    -         * - if not restoring, runs a `prepare`
    -         * - if just added, installsPluginsForNewPlatform (TODO for fil: familiarize
yourself why not just a regular "install plugin for platform" - why the 'newPlatform' API?)
    -         * - if not restoring, run a prepare. TODO: didnt we just do this? we just installed
plugins, so maybe its needed, but couldnt we just run a single prepare after possibly installing
plugins?
    -         * third `then`:
    -         * - save particular platform version installed to platform metadata.
    -         * - if autosaving or opts.save is provided, write platform to config.xml
    -         * fourth `then`:
    -         * - save added platform to package.json if exists.
    -         * fifth `then`:
    -         * - fire after_platform_add/update hook
    -         */
    +        it('should fire the before_platform_* hook', function () {
    +            platform_addHelper('add', hooks_mock, projectRoot, ['atari']);
    +            expect(hooks_mock.fire).toHaveBeenCalledWith('before_platform_add', jasmine.any(Object));
    +        });
    +
    +        it('should warn about using deprecated platforms', function (done) {
    +            spyOn(events, 'emit');
    +            platform_addHelper('add', hooks_mock, projectRoot, ['ubuntu', 'blackberry10']);
    +            process.nextTick(function () {
    +                expect(events.emit).toHaveBeenCalledWith(jasmine.stringMatching(/has
been deprecated/));
    +                done();
    +            });
    +        });
    +        describe('platform spec inference', function () {
    +            beforeEach(function () {
    +                spyOn(cordova, 'prepare').and.returnValue(Q());
    +                spyOn(prepare, 'preparePlatforms').and.returnValue(Q());
    --- End diff --
    
    Don't think you need these two spyOn's anymore since they are inline requires


> 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