[ https://issues.apache.org/jira/browse/CB-12361?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16068859#comment-16068859 ]
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_r124891852 --- 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()); + }); + + xit('should invoke the createPlatform platform API method when adding a platform, providing destination location, parsed config file and platform detail options as arguments', function (done) { + platform_addHelper('add', hooks_mock, projectRoot, ['ios'], {save: true, fetch: true}).then(function(result) { + expect(platform_api_mock.createPlatform).toHaveBeenCalled(); + }).fail(function (err) { + fail('unexpected failure handler invoked!'); + console.error(err); + }).done(done); + }); + + xit('should invoke the update platform API method when updating a platform, providing destination location and plaform detail options as arguments', function(done) { + spyOn(cordova_util, 'requireNoCache').and.returnValue({}); + cordova_util.isDirectory.and.returnValue(true); + fs.existsSync.and.returnValue(true); + platform_addHelper('update', hooks_mock, projectRoot, ['ios']).then(function(result) { + expect(platform_api_mock.updatePlatform).toHaveBeenCalled(); + }).fail(function (err) { + fail('unexpected failure handler invoked!'); + console.error(err); + }).done(done); + }); + }); + + describe('after platform api invocation', function () { + beforeEach(function () { + spyOn(cordova, 'prepare').and.returnValue(Q()); + spyOn(prepare, 'preparePlatforms').and.returnValue(Q()); + }); + + describe('when the restoring option is not provided', function () { --- End diff -- This one you need to keep `xit` since restoring option is not provided > 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-unsubscr...@cordova.apache.org For additional commands, e-mail: issues-h...@cordova.apache.org