Mailing-List: contact dev-help@cordova.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cordova.apache.org
From: stevengill <git@git.apache.org>
To: dev@cordova.apache.org
Reply-To: dev@cordova.apache.org
References: <git-pr-573-cordova-lib@git.apache.org>
In-Reply-To: <git-pr-573-cordova-lib@git.apache.org>
Subject: [GitHub] cordova-lib pull request #573: CB-12361 : updated addHelper tests
Content-Type: text/plain
Message-Id: <20170629194040.7855CE97E7@git1-us-west.apache.org>
Date: Thu, 29 Jun 2017 19:40:40 +0000 (UTC)
archived-at: Thu, 29 Jun 2017 19:40:46 -0000

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

    https://github.com/apache/cordova-lib/pull/573#discussion_r124880319
  
    --- 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());
    +            });
    +
    +            xit('should retrieve platform details from directories-specified-as-platforms using getPlatformDetailsFromDir', function (done) {
    +                cordova_util.isDirectory.and.returnValue(true);
    +                var directory_to_platform = '/path/to/cordova-atari';
    +                platform_addHelper('add', hooks_mock, projectRoot, [directory_to_platform]).then(function () {
    +                    expect(platform_module.getPlatformDetailsFromDir).toHaveBeenCalledWith(directory_to_platform, null);
    +                    expect(platform_addHelper.downloadPlatform).not.toHaveBeenCalled();
    +                }).fail(function (e) {
    +                    fail('fail handler unexpectedly invoked');
    +                    console.error(e);
    +                }).done(done);
    +            });
    +
    +            xit('should retrieve platform details from URLs-specified-as-platforms using downloadPlatform', function (done) {
    +                cordova_util.isUrl.and.returnValue(true);
    +                var url_to_platform = 'http://github.com/apache/cordova-atari';
    +                platform_addHelper('add', hooks_mock, projectRoot, [url_to_platform]).then(function () {
    +                    expect(platform_addHelper.downloadPlatform).toHaveBeenCalledWith(projectRoot, null, url_to_platform, jasmine.any(Object));
    +                }).fail(function (e) {
    +                    fail('fail handler unexpectedly invoked');
    +                    console.error(e);
    +                }).done(done);
    +            });
    +
    +            xit('should attempt to retrieve from config.xml if exists and package.json does not', function (done) {
    +                platform_addHelper('add', hooks_mock, projectRoot, ['atari']).then(function() {
    +                    expect(platform_addHelper.getVersionFromConfigFile).toHaveBeenCalled();
    +                }).fail(function (e) {
    +                    fail('fail handler unexpectedly invoked');
    +                    console.error(e);
    +                }).done(done);
    +            });
    +
    +            xit('should fall back to using pinned version if both package.json and config.xml do not specify it', function (done) {
    +                spyOn(events,'emit');
    +                platform_addHelper('add', hooks_mock, projectRoot, ['ios']).then(function() {
    +                    expect(events.emit.calls.argsFor(1)[1]).toBe('Grabbing pinned version.');
    --- End diff --
    
    use `toHaveBeenCalledWith()`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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