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

    https://github.com/apache/cordova-lib/pull/573#discussion_r124881373
  
    --- 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.');
    +                }).fail(function (e) {
    +                    fail('fail handler unexpectedly invoked');
    +                    console.error(e);
    +                }).done(done);
    +            });
    +
    +            xit('should invoke fetch if provided as an option and spec is 
a directory', function (done) {
    +                cordova_util.isDirectory.and.returnValue(projectRoot);
    +                cordova_util.fixRelativePath.and.returnValue(projectRoot);
    +                spyOn(path, 'resolve').and.callThrough();
    +                platform_addHelper('add', hooks_mock, projectRoot, 
['ios'], {save:true, fetch: true}).then(function() {
    +                    expect(path.resolve).toHaveBeenCalled();
    +                }).fail(function (e) {
    +                    fail('fail handler unexpectedly invoked');
    +                    console.error(e);
    +                }).done(done);
    +            });
    +        });
    +
    +        describe('platform api invocation', function () {
    +            beforeEach(function () {
    +                spyOn(cordova, 'prepare').and.returnValue(Q());
    +                spyOn(prepare, 'preparePlatforms').and.returnValue(Q());
    --- End diff --
    
    don't need these spyOn's


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

Reply via email to