You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by audreyso <gi...@git.apache.org> on 2016/12/14 19:49:03 UTC

[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

GitHub user audreyso opened a pull request:

    https://github.com/apache/cordova-lib/pull/510

    Fixjasmine : CB:12018 - updating tests in cordova-lib to use jasmine instead of jasmine-node

    <!--
    Please make sure the checklist boxes are all checked before submitting the PR. The checklist
    is intended as a quick reference, for complete details please see our Contributor Guidelines:
    
    http://cordova.apache.org/contribute/contribute_guidelines.html
    
    Thanks!
    -->
    
    ### Platforms affected
    
    
    ### What does this PR do?
    
    
    ### What testing has been done on this change?
    
    
    ### Checklist
    - [X] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database
    - [X] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
    - [X] Added automated test coverage as appropriate for this change.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/audreyso/cordova-lib fixjasmine

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cordova-lib/pull/510.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #510
    
----
commit bda891918bdc8ad172a08090a5325a16113f659e
Author: audreyso <au...@adobe.com>
Date:   2016-12-14T17:33:47Z

    fixjasmine : CB-12018 modifying to switch from jasmine node to jasmine

commit 75a666dccb67398b3ae01fe29a016bb928d8c507
Author: audreyso <au...@adobe.com>
Date:   2016-12-14T17:36:28Z

    fixjasmine : CB-12018 : updated tests to work with jasmine

commit 767af3392b797f803550d2a07ac87c8a022738f3
Author: audreyso <au...@adobe.com>
Date:   2016-12-14T18:27:13Z

     fixjasmine : CB-12018 : updated tests in spec-cordova to function with jasmine

commit ef096b83814e3bed88be62945ba86c74d7763bc5
Author: audreyso <au...@adobe.com>
Date:   2016-12-14T18:55:34Z

    fixjasmine : CB:12018 : updated plugin_fetch to work with jasmine testing

commit d3eb9ae6a11f4be7429e77366c5a79dd8af57f2e
Author: audreyso <au...@adobe.com>
Date:   2016-12-14T19:17:43Z

     fixjasmine : CB-12018 : updated spec-plugman tests to work with jasmine (spyOn and return, callThrough, callFake, describe)

commit b0e427c675dc6f0d112cb91c961780b27677a8c2
Author: audreyso <au...@adobe.com>
Date:   2016-12-14T19:45:17Z

    fixjasmine : CB-12018 : updated cordova-common/spec to work with jasmine (spyOn, return, callFake, this.after)

----


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by audreyso <gi...@git.apache.org>.
Github user audreyso commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r92480348
  
    --- Diff: cordova-common/spec/CordovaCheck.spec.js ---
    @@ -30,11 +30,12 @@ describe('findProjectRoot method', function() {
             process.env.PWD = origPWD;
             process.chdir(cwd);
         });
    +function removeDir(someDirectory) {
    +    shell.rm('-rf', someDirectory);
    +}
         it('should return false if it hits the home directory', function() {
             var somedir = path.join(home, 'somedir');
    -        this.after(function() {
    --- End diff --
    
    Here does this.after remove the somedir similarly to how we are using in for testing save/fetch? In the beforeEach, I have ``` shell.rm('-rf', project); ``` to clear the previous temp project? Maybe I should just use my function in a beforeEach?  But then how will the someDir get updated if it changes from test to test? (if that makes sense)


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95500793
  
    --- Diff: cordova-lib/spec-cordova/plugin.spec.js ---
    @@ -216,91 +229,110 @@ describe('plugin end-to-end', function() {
                 expect(defaultPluginPreferences.REQUIRED).toBe('NO');
                 return removePlugin(org_test_defaultvariables);
            })
    -       .fail(errorHandler.errorCallback)
    -       .fin(done);
    -    });
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
    +        .fin(done);
    +    }, 30000);
     
    -    it('should successfully add a plugin when specifying CLI variables', function(done) {
    +    it('Test 006 : should successfully add a plugin when specifying CLI variables', function(done) {
             addPlugin(path.join(pluginsDir, org_test_defaultvariables), org_test_defaultvariables, {cli_variables: { REQUIRED:'yes', REQUIRED_ANDROID:'yes'}}, done)
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    --- End diff --
    
    same as 155


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by audreyso <gi...@git.apache.org>.
Github user audreyso commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95625647
  
    --- Diff: cordova-lib/spec-plugman/platforms/windows.spec.js ---
    @@ -192,34 +203,39 @@ beforeEach(function () {
     
                     projectsAddedToSpies.forEach(function (spy) {
                         expect(spy).toHaveBeenCalled();
    +                    spy.calls.reset();
    --- End diff --
    
    I took out all of the resets.. everything still seems to be working when I test the file individually and also when I run npm test.


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95499196
  
    --- Diff: cordova-common/spec/CordovaCheck.spec.js ---
    @@ -30,71 +30,62 @@ describe('findProjectRoot method', function() {
             process.env.PWD = origPWD;
             process.chdir(cwd);
         });
    -    it('should return false if it hits the home directory', function() {
    +function removeDir(someDirectory) {
    --- End diff --
    
    I think this needs to be indented


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95502073
  
    --- Diff: cordova-lib/spec-plugman/platforms/windows.spec.js ---
    @@ -192,34 +203,39 @@ beforeEach(function () {
     
                     projectsAddedToSpies.forEach(function (spy) {
                         expect(spy).toHaveBeenCalled();
    +                    spy.calls.reset();
    --- End diff --
    
    don't think we need the reset call anymore. Same for line 211


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95500754
  
    --- Diff: cordova-lib/spec-cordova/plugin.spec.js ---
    @@ -199,11 +209,14 @@ describe('plugin end-to-end', function() {
             .then(function() {
                 return removePlugin(pluginId);
             })
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    --- End diff --
    
    same as 155


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r92901030
  
    --- Diff: cordova-lib/spec-plugman/install.spec.js ---
    @@ -345,13 +344,14 @@ describe('install', function() {
     
             it('should not check custom engine version that is not supported for platform', function() {
                 var spy = spyOn(semver, 'satisfies').and.returnValue(true);
    -            runs(function() {
    -                installPromise( install('blackberry10', project, plugins['com.cordova.engine']) );
    -            });
    -            waitsFor(function() { return done; }, 'install promise never resolved', 200);
    -            runs(function() {
    +            install('blackberry10', project, plugins['com.cordova.engine']).then(function(done) {
    +                expect(false).toBe(true);
    +                done();
    +            },
    +            function err(errMsg) {
                     expect(spy).not.toHaveBeenCalledWith('','>=3.0.0');
    --- End diff --
    
    is this being run? Why call it err and pass errMsg? You don't seem to be using errMsg


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by audreyso <gi...@git.apache.org>.
Github user audreyso commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r92707427
  
    --- Diff: cordova-lib/spec-cordova/build.spec.js ---
    @@ -67,7 +67,7 @@ describe('build command', function() {
         describe('success', function() {
             it('should run inside a Cordova-based project with at least one added platform and call both prepare and compile', function(done) {
                 cordova.raw.build(['android','ios']).then(function() {
    -                var opts = {verbose: false, platforms: ['android', 'ios'], options: []};
    +                var opts = Object({ platforms: [ 'android', 'ios' ], verbose: false, options: Object({  }) })
    --- End diff --
    
    Wasn't sure about this change because the expect was failing... and expecting what I changed here


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95500805
  
    --- Diff: cordova-lib/spec-cordova/plugin.spec.js ---
    @@ -216,91 +229,110 @@ describe('plugin end-to-end', function() {
                 expect(defaultPluginPreferences.REQUIRED).toBe('NO');
                 return removePlugin(org_test_defaultvariables);
            })
    -       .fail(errorHandler.errorCallback)
    -       .fin(done);
    -    });
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
    +        .fin(done);
    +    }, 30000);
     
    -    it('should successfully add a plugin when specifying CLI variables', function(done) {
    +    it('Test 006 : should successfully add a plugin when specifying CLI variables', function(done) {
             addPlugin(path.join(pluginsDir, org_test_defaultvariables), org_test_defaultvariables, {cli_variables: { REQUIRED:'yes', REQUIRED_ANDROID:'yes'}}, done)
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
             .fin(done);
    -    });
    +    }, 30000);
     
    -    it('should not check npm info when using the searchpath flag', function(done) {
    +    it('Test 007 : should not check npm info when using the searchpath flag', function(done) {
             mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin));
    -
             spyOn(registry, 'info');
    -        addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {searchpath: pluginsDir}, done)
    +        return addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {searchpath: pluginsDir}, done)
             .then(function() {
                 expect(registry.info).not.toHaveBeenCalled();
    -
    -            var fetchOptions = plugman.raw.fetch.mostRecentCall.args[2];
    -            expect(fetchOptions.searchpath).toBeDefined();
    +            var fetchOptions = plugman.raw.fetch.calls.mostRecent().args[2];
    +            expect(fetchOptions.searchpath[0]).toExist();
    +        })
    +        .fail(function(err) {
    +            console.error(err);
    --- End diff --
    
    same as 155


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95501588
  
    --- Diff: cordova-lib/spec-plugman/install.spec.js ---
    @@ -206,134 +178,136 @@ describe('install', function() {
     
         beforeEach(function() {
     
    -        exec = spyOn(child_process, 'exec').andCallFake(function(cmd, cb) {
    +        exec = spyOn(child_process, 'exec').and.callFake(function(cmd, cb) {
                 cb(false, '', '');
             });
    -        spawnSpy = spyOn(superspawn, 'spawn').andReturn(Q('3.1.0'));
    -        spyOn(fs, 'mkdirSync').andReturn(true);
    -        spyOn(shell, 'mkdir').andReturn(true);
    -        spyOn(platforms, 'copyFile').andReturn(true);
    -
    -        fetchSpy = spyOn(plugman.raw, 'fetch').andReturn( Q( plugins['com.cordova.engine'] ) );
    -        chmod = spyOn(fs, 'chmodSync').andReturn(true);
    -        spyOn(fs, 'writeFileSync').andReturn(true);
    -        cp = spyOn(shell, 'cp').andReturn(true);
    -        rm = spyOn(shell, 'rm').andReturn(true);
    +        spawnSpy = spyOn(superspawn, 'spawn').and.returnValue(Q('3.1.0'));
    +        spyOn(fs, 'mkdirSync').and.returnValue(true);
    +        spyOn(shell, 'mkdir').and.returnValue(true);
    +        spyOn(platforms, 'copyFile').and.returnValue(true);
    +
    +        fetchSpy = spyOn(plugman.raw, 'fetch').and.returnValue( Q( plugins['com.cordova.engine'] ) );
    +        chmod = spyOn(fs, 'chmodSync').and.returnValue(true);
    +        spyOn(fs, 'writeFileSync').and.returnValue(true);
    +        cp = spyOn(shell, 'cp').and.returnValue(true);
    +        rm = spyOn(shell, 'rm').and.returnValue(true);
             add_to_queue = spyOn(PlatformJson.prototype, 'addInstalledPluginToPrepareQueue');
             done = false;
         });
     
         describe('success', function() {
    -        it('should emit a results event with platform-agnostic <info>', function() {
    +        it('Test 002 : should emit a results event with platform-agnostic <info>', function() {
                 // org.test.plugins.childbrowser
                 expect(results['emit_results'][0]).toBe('No matter what platform you are installing to, this notice is very important.');
             });
    -        it('should emit a results event with platform-specific <info>', function() {
    +        it('Test 003 : should emit a results event with platform-specific <info>', function() {
                 // org.test.plugins.childbrowser
                 expect(results['emit_results'][1]).toBe('Please make sure you read this because it is very important to complete the installation of your plugin.');
             });
    -        it('should interpolate variables into <info> tags', function() {
    +        it('Test 004 : should interpolate variables into <info> tags', function() {
                 // VariableBrowser
                 expect(results['emit_results'][2]).toBe('Remember that your api key is batman!');
             });
    -        it('should call fetch if provided plugin cannot be resolved locally', function() {
    -            fetchSpy.andReturn( Q( plugins['org.test.plugins.dummyplugin'] ) );
    -            spyOn(fs, 'existsSync').andCallFake( fake['existsSync']['noPlugins'] );
    -
    -            runs(function() {
    -                installPromise(install('android', project, 'CLEANYOURSHORTS' ));
    -            });
    -            waitsFor(function() { return done; }, 'install promise never resolved', 200);
    -            runs(function() {
    -                expect(done).toBe(true);
    +        it('Test 005 : should call fetch if provided plugin cannot be resolved locally', function(done) {
    +            fetchSpy.and.returnValue( Q( plugins['org.test.plugins.dummyplugin'] ) );
    +            spyOn(fs, 'existsSync').and.callFake( fake['existsSync']['noPlugins'] );
    +            install('android', project, 'CLEANYOURSHORTS')
    +            .fail(function(err){
    +                console.log(err);
    --- End diff --
    
    Maybe expect err to be undefined or something. No need to console.log it


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r93490653
  
    --- Diff: cordova-lib/spec-plugman/platforms/windows.spec.js ---
    @@ -50,18 +50,30 @@ function copyArray(arr) {
     }
     
     beforeEach(function () {
    -    this.addMatchers({
    +    jasmine.addMatchers({
             toContainXmlPath: function (xpath) {
                 var xml = this.actual;
                 var notText = this.isNot ? 'not ' : '';
    --- End diff --
    
    delete this


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r93490083
  
    --- Diff: cordova-lib/spec-plugman/platforms/ios.spec.js ---
    @@ -301,14 +301,8 @@ describe('ios project handler', function() {
                 });
             });
             it('of two plugins should apply xcode file changes from both', function(){
    --- End diff --
    
    I think you should add `done` here and call it at the end of the promise. See what happens. 
    
    You can also chain the two promises (move 315 into 313). 
    
    Add some console logs to confirm these tests are actually running


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95500893
  
    --- Diff: cordova-lib/spec-cordova/plugin.spec.js ---
    @@ -309,10 +341,13 @@ describe('plugin end-to-end', function() {
             .then(function() {
                 expect(registry.info).not.toHaveBeenCalled();
     
    -            var fetchTarget = plugman.raw.fetch.mostRecentCall.args[0];
    +            var fetchTarget = plugman.raw.fetch.calls.mostRecent().args[0];
                 expect(fetchTarget).toEqual(scopedPackage);
             })
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    --- End diff --
    
    same as 155


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r92756760
  
    --- Diff: cordova-lib/spec-cordova/helpers.js ---
    @@ -148,7 +148,7 @@ module.exports.writeConfigContent = function (appPath, configContent) {
     
     // Add the toExist matcher.
     beforeEach(function () {
    -    this.addMatchers({
    +    jasmine.addMatchers({
             'toExist': function () {
    --- End diff --
    
    I rewrote `toExist` with the new [custom matcher syntax](https://jasmine.github.io/2.0/custom_matcher.html). This should fix some of your failing specs. 
    
    Replace this `toExist` with:
    
    ```
    jasmine.addMatchers({
            'toExist': function () {
                return {
                    compare: function(actual, expected) {
                        var notText = this.isNot ? ' not' : '';
                        var result = {};
    
                        result.pass = fs.existsSync(actual);
    
                        if(result.pass) {
                            result.message = "expected " + actual + " to not exist";
                        } else {
                            result.message = "expected " + actual + " to exist";
                        }
    
                        return result;
                    }
                }
            }
        });
    ```
    



---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r93491264
  
    --- Diff: cordova-lib/spec-plugman/platforms/windows.spec.js ---
    @@ -50,18 +50,30 @@ function copyArray(arr) {
     }
     
     beforeEach(function () {
    -    this.addMatchers({
    +    jasmine.addMatchers({
             toContainXmlPath: function (xpath) {
                 var xml = this.actual;
    --- End diff --
    
    Move this onto line 62 (inside compare function)
    
    ```
    var xml = actual;
    ```


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95500848
  
    --- Diff: cordova-lib/spec-cordova/plugin.spec.js ---
    @@ -216,91 +229,110 @@ describe('plugin end-to-end', function() {
                 expect(defaultPluginPreferences.REQUIRED).toBe('NO');
                 return removePlugin(org_test_defaultvariables);
            })
    -       .fail(errorHandler.errorCallback)
    -       .fin(done);
    -    });
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
    +        .fin(done);
    +    }, 30000);
     
    -    it('should successfully add a plugin when specifying CLI variables', function(done) {
    +    it('Test 006 : should successfully add a plugin when specifying CLI variables', function(done) {
             addPlugin(path.join(pluginsDir, org_test_defaultvariables), org_test_defaultvariables, {cli_variables: { REQUIRED:'yes', REQUIRED_ANDROID:'yes'}}, done)
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
             .fin(done);
    -    });
    +    }, 30000);
     
    -    it('should not check npm info when using the searchpath flag', function(done) {
    +    it('Test 007 : should not check npm info when using the searchpath flag', function(done) {
             mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin));
    -
             spyOn(registry, 'info');
    -        addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {searchpath: pluginsDir}, done)
    +        return addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {searchpath: pluginsDir}, done)
             .then(function() {
                 expect(registry.info).not.toHaveBeenCalled();
    -
    -            var fetchOptions = plugman.raw.fetch.mostRecentCall.args[2];
    -            expect(fetchOptions.searchpath).toBeDefined();
    +            var fetchOptions = plugman.raw.fetch.calls.mostRecent().args[2];
    +            expect(fetchOptions.searchpath[0]).toExist();
    +        })
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
             })
    -        .fail(errorHandler.errorCallback)
             .fin(done);
    -    });
    +    }, 30000);
     
    -    it('should not check npm info when using the noregistry flag', function(done) {
    +    it('Test 008 : should not check npm info when using the noregistry flag', function(done) {
             mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin));
     
             spyOn(registry, 'info');
             addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {noregistry:true}, done)
             .then(function() {
                 expect(registry.info).not.toHaveBeenCalled();
     
    -            var fetchOptions = plugman.raw.fetch.mostRecentCall.args[2];
    +            var fetchOptions = plugman.raw.fetch.calls.mostRecent().args[2];
                 expect(fetchOptions.noregistry).toBeTruthy();
             })
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
             .fin(done);
    -    });
    +    }, 30000);
     
    -    it('should not check npm info when fetching from a Git repository', function(done) {
    +    it('Test 009 : should not check npm info when fetching from a Git repository', function(done) {
             spyOn(registry, 'info');
             addPlugin(testGitPluginRepository, testGitPluginId, {}, done)
             .then(function() {
                 expect(registry.info).not.toHaveBeenCalled();
             })
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    --- End diff --
    
    same as 155


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95500695
  
    --- Diff: cordova-lib/spec-cordova/plugin.spec.js ---
    @@ -145,30 +146,36 @@ describe('plugin end-to-end', function() {
             expect(errorHandler.errorCallback).not.toHaveBeenCalled();
         });
     
    -    it('should successfully add and remove a plugin with no options', function(done) {
    +    it('Test 001 : should successfully add and remove a plugin with no options', function(done) {
             addPlugin(path.join(pluginsDir, 'fake1'), pluginId, {}, done)
             .then(function() {
                 return removePlugin(pluginId);
             })
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    --- End diff --
    
    don't need to console.error this


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95500869
  
    --- Diff: cordova-lib/spec-cordova/plugin.spec.js ---
    @@ -216,91 +229,110 @@ describe('plugin end-to-end', function() {
                 expect(defaultPluginPreferences.REQUIRED).toBe('NO');
                 return removePlugin(org_test_defaultvariables);
            })
    -       .fail(errorHandler.errorCallback)
    -       .fin(done);
    -    });
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
    +        .fin(done);
    +    }, 30000);
     
    -    it('should successfully add a plugin when specifying CLI variables', function(done) {
    +    it('Test 006 : should successfully add a plugin when specifying CLI variables', function(done) {
             addPlugin(path.join(pluginsDir, org_test_defaultvariables), org_test_defaultvariables, {cli_variables: { REQUIRED:'yes', REQUIRED_ANDROID:'yes'}}, done)
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
             .fin(done);
    -    });
    +    }, 30000);
     
    -    it('should not check npm info when using the searchpath flag', function(done) {
    +    it('Test 007 : should not check npm info when using the searchpath flag', function(done) {
             mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin));
    -
             spyOn(registry, 'info');
    -        addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {searchpath: pluginsDir}, done)
    +        return addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {searchpath: pluginsDir}, done)
             .then(function() {
                 expect(registry.info).not.toHaveBeenCalled();
    -
    -            var fetchOptions = plugman.raw.fetch.mostRecentCall.args[2];
    -            expect(fetchOptions.searchpath).toBeDefined();
    +            var fetchOptions = plugman.raw.fetch.calls.mostRecent().args[2];
    +            expect(fetchOptions.searchpath[0]).toExist();
    +        })
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
             })
    -        .fail(errorHandler.errorCallback)
             .fin(done);
    -    });
    +    }, 30000);
     
    -    it('should not check npm info when using the noregistry flag', function(done) {
    +    it('Test 008 : should not check npm info when using the noregistry flag', function(done) {
             mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin));
     
             spyOn(registry, 'info');
             addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {noregistry:true}, done)
             .then(function() {
                 expect(registry.info).not.toHaveBeenCalled();
     
    -            var fetchOptions = plugman.raw.fetch.mostRecentCall.args[2];
    +            var fetchOptions = plugman.raw.fetch.calls.mostRecent().args[2];
                 expect(fetchOptions.noregistry).toBeTruthy();
             })
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
             .fin(done);
    -    });
    +    }, 30000);
     
    -    it('should not check npm info when fetching from a Git repository', function(done) {
    +    it('Test 009 : should not check npm info when fetching from a Git repository', function(done) {
             spyOn(registry, 'info');
             addPlugin(testGitPluginRepository, testGitPluginId, {}, done)
             .then(function() {
                 expect(registry.info).not.toHaveBeenCalled();
             })
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
             .fin(done);
    -    });
    +    }, 30000);
     
    -    it('should select the plugin version based on npm info when fetching from npm', function(done) {
    +    it('Test 010 : should select the plugin version based on npm info when fetching from npm', function(done) {
             mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin));
     
    -        spyOn(registry, 'info').andCallThrough();
    +        spyOn(registry, 'info').and.callThrough();
             addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {}, done)
             .then(function() {
                 expect(registry.info).toHaveBeenCalled();
     
    -            var fetchTarget = plugman.raw.fetch.mostRecentCall.args[0];
    +            var fetchTarget = plugman.raw.fetch.calls.mostRecent().args[0];
                 expect(fetchTarget).toEqual(npmInfoTestPlugin + '@' + npmInfoTestPluginVersion);
             })
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
             .fin(done);
    -    });
    +    }, 30000);
     
    -    it('should handle scoped npm packages', function(done) {
    +    it('Test 011 : should handle scoped npm packages', function(done) {
             var scopedPackage = '@testscope/' + npmInfoTestPlugin;
             mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin));
     
    -        spyOn(registry, 'info').andReturn(Q({}));
    +        spyOn(registry, 'info').and.returnValue(Q({}));
             addPlugin(scopedPackage, npmInfoTestPlugin, {}, done)
             .then(function() {
                 // Check to make sure that we are at least trying to get the correct package.
                 // This package is not published to npm, so we can't truly do end-to-end tests
     
                 expect(registry.info).toHaveBeenCalledWith([scopedPackage]);
     
    -            var fetchTarget = plugman.raw.fetch.mostRecentCall.args[0];
    +            var fetchTarget = plugman.raw.fetch.calls.mostRecent().args[0];
                 expect(fetchTarget).toEqual(scopedPackage);
             })
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    --- End diff --
    
    same as 155


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r92901285
  
    --- Diff: cordova-lib/spec-plugman/install.spec.js ---
    @@ -345,13 +344,14 @@ describe('install', function() {
     
             it('should not check custom engine version that is not supported for platform', function() {
                 var spy = spyOn(semver, 'satisfies').and.returnValue(true);
    -            runs(function() {
    -                installPromise( install('blackberry10', project, plugins['com.cordova.engine']) );
    -            });
    -            waitsFor(function() { return done; }, 'install promise never resolved', 200);
    -            runs(function() {
    +            install('blackberry10', project, plugins['com.cordova.engine']).then(function(done) {
    +                expect(false).toBe(true);
    +                done();
    +            },
    +            function err(errMsg) {
                     expect(spy).not.toHaveBeenCalledWith('','>=3.0.0');
    --- End diff --
    
    on line 348, your purposely getting it to fail?


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r93490510
  
    --- Diff: cordova-lib/spec-plugman/platforms/windows.spec.js ---
    @@ -50,18 +50,30 @@ function copyArray(arr) {
     }
     
     beforeEach(function () {
    -    this.addMatchers({
    +    jasmine.addMatchers({
             toContainXmlPath: function (xpath) {
                 var xml = this.actual;
                 var notText = this.isNot ? 'not ' : '';
    -            this.message = function () {
    -                return 'Expected xml \'' + et.tostring(xml) + '\' ' + notText + 'to contain elements matching \'' + xpath + '\'.';
    -            };
    -
    -            return xml.find(xpath) !== null;
    -        }    });
    +            // this.message = function () {
    +            //     return 'Expected xml \'' + et.tostring(xml) + '\' ' + notText + 'to contain elements matching \'' + xpath + '\'.';
    +            // };
    +            return {
    +                compare: function(actual, expected) {
    +                    var result = {};
    +                    result.pass = fs.existsSync(actual);
    --- End diff --
    
    replace this with 
    ```
     result.pass = xml.find(expected) !== null;
    ```


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95500649
  
    --- Diff: cordova-lib/spec-cordova/plugin.spec.js ---
    @@ -83,6 +83,7 @@ function removePlugin(id) {
     var errorHandler = {
         errorCallback: function(error) {
             // We want the error to be printed by jasmine
    +        console.log(error);
    --- End diff --
    
    you can remove this console.log


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95500766
  
    --- Diff: cordova-lib/spec-cordova/plugin.spec.js ---
    @@ -216,91 +229,110 @@ describe('plugin end-to-end', function() {
                 expect(defaultPluginPreferences.REQUIRED).toBe('NO');
                 return removePlugin(org_test_defaultvariables);
            })
    -       .fail(errorHandler.errorCallback)
    -       .fin(done);
    -    });
    +        .fail(function(err) {
    +            console.error(err);
    --- End diff --
    
    same as 155


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95500742
  
    --- Diff: cordova-lib/spec-cordova/plugin.spec.js ---
    @@ -178,11 +185,14 @@ describe('plugin end-to-end', function() {
             .then(function () {
                 expect(prepare.preparePlatforms).not.toHaveBeenCalled();
             })
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    --- End diff --
    
    same as 155


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95502237
  
    --- Diff: cordova-lib/spec-plugman/platforms/windows.spec.js ---
    @@ -192,34 +203,39 @@ beforeEach(function () {
     
                     projectsAddedToSpies.forEach(function (spy) {
                         expect(spy).toHaveBeenCalled();
    +                    spy.calls.reset();
    --- End diff --
    
    actually, not sure. Maybe try without and see what happens? You have two more resets in this file below


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r92707699
  
    --- Diff: cordova-common/package.json ---
    @@ -28,6 +28,7 @@
         "cordova-registry-mapper": "^1.1.8",
         "elementtree": "^0.1.6",
         "glob": "^5.0.13",
    +    "jasmine": "^2.5.2",
    --- End diff --
    
    can you move this down to devDependencies instead and also remove jasmine-node from there


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r92713582
  
    --- Diff: cordova-lib/spec-cordova/HooksRunner.spec.js ---
    @@ -472,7 +472,7 @@ describe('HooksRunner', function() {
                     hookOptions.nohooks = ['before_build'];
     
                     return hooksRunner.fire(test_event, hookOptions).then(function (msg) {
    -                    expect(msg).toBeDefined();
    +                    expect(msg).toExist();
    --- End diff --
    
    looks like some of these were originally toBeDefinied. 475, 512, 538, 548, 641


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by audreyso <gi...@git.apache.org>.
Github user audreyso commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95627089
  
    --- Diff: cordova-lib/spec-plugman/platforms/windows.spec.js ---
    @@ -192,34 +203,39 @@ beforeEach(function () {
     
                     projectsAddedToSpies.forEach(function (spy) {
                         expect(spy).toHaveBeenCalled();
    +                    spy.calls.reset();
    --- End diff --
    
    Thanks @stevengill ! All comments have been addressed. Let me know if there is anything else!


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95500726
  
    --- Diff: cordova-lib/spec-cordova/plugin.spec.js ---
    @@ -145,30 +146,36 @@ describe('plugin end-to-end', function() {
             expect(errorHandler.errorCallback).not.toHaveBeenCalled();
         });
     
    -    it('should successfully add and remove a plugin with no options', function(done) {
    +    it('Test 001 : should successfully add and remove a plugin with no options', function(done) {
             addPlugin(path.join(pluginsDir, 'fake1'), pluginId, {}, done)
             .then(function() {
                 return removePlugin(pluginId);
             })
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
             .fin(done);
    -    });
    +    }, 30000);
     
    -    it('should run prepare after plugin installation/removal by default', function(done) {
    +    it('Test 002 : should run prepare after plugin installation/removal by default', function(done) {
             addPlugin(path.join(pluginsDir, 'fake1'), pluginId, {})
             .then(function() {
                 expect(prepare.preparePlatforms).toHaveBeenCalled();
    -            prepare.preparePlatforms.reset();
    +            prepare.preparePlatforms.calls.reset();
                 return removePlugin(pluginId);
             })
             .then(function () {
                 expect(prepare.preparePlatforms).toHaveBeenCalled();
             })
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    --- End diff --
    
    same as 155


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by audreyso <gi...@git.apache.org>.
Github user audreyso commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r93493946
  
    --- Diff: cordova-lib/spec-plugman/platforms/ios.spec.js ---
    @@ -301,14 +301,8 @@ describe('ios project handler', function() {
                 });
             });
             it('of two plugins should apply xcode file changes from both', function(){
    --- End diff --
    
    @stevengill : if I modify this test, the console.logs both print out, but there are 3 failures.
    
    ``` it('of two plugins should apply xcode file changes from both', function(done){
                 install('ios', temp, dummyplugin)
                .then(function (result) { 
                    var xcode = ios.parseProjectFile(temp).xcode;
                    // from org.test.plugins.dummyplugin
                    expect(xcode.hasFile(slashJoin('Resources', 'DummyPlugin.bundle'))).toBeTruthy();
                    expect(xcode.hasFile(slashJoin('Plugins','org.test.plugins.dummyplugin', 'DummyPluginCommand.h'))).toBeTruthy();
                    expect(xcode.hasFile(slashJoin('Plugins','org.test.plugins.dummyplugin', 'DummyPluginCommand.m'))).toBeTruthy();
                    expect(xcode.hasFile(slashJoin('Plugins','org.test.plugins.dummyplugin','targetDir','TargetDirTest.h'))).toBeTruthy();
                    expect(xcode.hasFile(slashJoin('Plugins','org.test.plugins.dummyplugin','targetDir','TargetDirTest.m'))).toBeTruthy();
                    expect(xcode.hasFile(slashJoin('SampleApp','Plugins','org.test.plugins.dummyplugin','Custom.framework'))).toBeTruthy();
                    console.log('done 1')
                }).then(function (result) {
                    install('ios', temp, weblessplugin)
                }).then(function (result) {
                    var xcode = ios.parseProjectFile(temp).xcode;
                    // from org.test.plugins.weblessplugin
                    // Below fails and wants false
                    expect(xcode.hasFile(slashJoin('Resources', 'WeblessPluginViewController.xib'))).toBeTruthy();
                    // Below fails and wants false
                    expect(xcode.hasFile(slashJoin('Plugins','org.test.plugins.weblessplugin','WeblessPluginCommand.h'))).toBeTruthy();
                    // Below fails and wants false
                    expect(xcode.hasFile(slashJoin('Plugins','org.test.plugins.weblessplugin','WeblessPluginCommand.m'))).toBeTruthy();
                    expect(xcode.hasFile('usr/lib/libsqlite3.dylib')).toBeTruthy();
                    console.log('done 2')
                });
                done();
            });
        }); ```


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95497591
  
    --- Diff: cordova-common/spec/ActionStack.spec.js ---
    @@ -59,19 +59,19 @@ describe('action-stack', function() {
                 stack.push(stack.createAction(third_spy, third_args, function(){}, []));
                 // process should throw
                 var error;
    -            runs(function() {
    -                stack.process('android', android_one_project).fail(function(err) { error = err; });
    -            });
    -            waitsFor(function(){ return error; }, 'process promise never resolved', 500);
    -            runs(function() {
    +            stack.process('android', android_one_project)
    +            .then(function(something){
    --- End diff --
    
    Don't need `something` as an argument here. 


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r93492030
  
    --- Diff: cordova-lib/spec-plugman/platforms/windows.spec.js ---
    @@ -50,18 +50,30 @@ function copyArray(arr) {
     }
     
     beforeEach(function () {
    -    this.addMatchers({
    +    jasmine.addMatchers({
             toContainXmlPath: function (xpath) {
                 var xml = this.actual;
                 var notText = this.isNot ? 'not ' : '';
    -            this.message = function () {
    -                return 'Expected xml \'' + et.tostring(xml) + '\' ' + notText + 'to contain elements matching \'' + xpath + '\'.';
    -            };
    -
    -            return xml.find(xpath) !== null;
    -        }    });
    +            // this.message = function () {
    +            //     return 'Expected xml \'' + et.tostring(xml) + '\' ' + notText + 'to contain elements matching \'' + xpath + '\'.';
    +            // };
    +            return {
    +                compare: function(actual, expected) {
    +                    var result = {};
    +                    result.pass = fs.existsSync(actual);
    +
    +            //return xml.find(xpath) !== null;
    +                    if(result.pass) {
    +                        result.message = "expected " + actual + " to not exist";
    --- End diff --
    
    Update first message to 
    `'Expected xml \'' + et.tostring(xml) + '\' ' +' not to contain elements matching \'' + expected + '\'.';`


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r92707789
  
    --- Diff: cordova-lib/package.json ---
    @@ -20,15 +20,16 @@
       "dependencies": {
         "aliasify": "^1.7.2",
         "cordova-common": "1.5.x",
    -    "cordova-fetch": "^1.0.1",
         "cordova-create": "^1.0.1",
    +    "cordova-fetch": "^1.0.1",
         "cordova-js": "4.2.0",
         "cordova-registry-mapper": "1.x",
         "cordova-serve": "^1.0.0",
         "dep-graph": "1.1.0",
         "elementtree": "0.1.6",
         "glob": "^5.0.3",
         "init-package-json": "^1.2.0",
    +    "jasmine": "^2.5.2",
    --- End diff --
    
    same as my previous comment


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by audreyso <gi...@git.apache.org>.
Github user audreyso commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r93502548
  
    --- Diff: cordova-lib/spec-plugman/platforms/ios.spec.js ---
    @@ -301,14 +301,8 @@ describe('ios project handler', function() {
                 });
             });
             it('of two plugins should apply xcode file changes from both', function(){
    --- End diff --
    
    @stevengill Does the order of the test matter? If I test this way, the test passes just fine.
    
    ``` it('of two plugins should apply xcode file changes from both', function(done){
                return install('ios', temp, weblessplugin)
                .then(function (result) { 
                    var xcode = ios.parseProjectFile(temp).xcode;
                    // from org.test.plugins.weblessplugin
                    expect(xcode.hasFile(slashJoin('Resources', 'WeblessPluginViewController.xib'))).toBeTruthy();
                    expect(xcode.hasFile(slashJoin('Plugins','org.test.plugins.weblessplugin','WeblessPluginCommand.h'))).toBeTruthy();
                    expect(xcode.hasFile(slashJoin('Plugins','org.test.plugins.weblessplugin','WeblessPluginCommand.m'))).toBeTruthy();
                    expect(xcode.hasFile('usr/lib/libsqlite3.dylib')).toBeTruthy();
                    done();
                }).then(function () {
                    return install('ios', temp, dummyplugin)
                }).then(function () {
                    var xcode = ios.parseProjectFile(temp).xcode;
                    // from org.test.plugins.dummyplugin
                    expect(xcode.hasFile(slashJoin('Resources', 'DummyPlugin.bundle'))).toBeTruthy();
                    expect(xcode.hasFile(slashJoin('Plugins','org.test.plugins.dummyplugin', 'DummyPluginCommand.h'))).toBeTruthy();
                    expect(xcode.hasFile(slashJoin('Plugins','org.test.plugins.dummyplugin', 'DummyPluginCommand.m'))).toBeTruthy();
                    expect(xcode.hasFile(slashJoin('Plugins','org.test.plugins.dummyplugin','targetDir','TargetDirTest.h'))).toBeTruthy();
                    expect(xcode.hasFile(slashJoin('Plugins','org.test.plugins.dummyplugin','targetDir','TargetDirTest.m'))).toBeTruthy();
                    expect(xcode.hasFile(slashJoin('SampleApp','Plugins','org.test.plugins.dummyplugin','Custom.framework'))).toBeTruthy();
                    done();
                });
            }, 60000);
        }); ```


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r93492132
  
    --- Diff: cordova-lib/spec-plugman/platforms/windows.spec.js ---
    @@ -50,18 +50,30 @@ function copyArray(arr) {
     }
     
     beforeEach(function () {
    -    this.addMatchers({
    +    jasmine.addMatchers({
             toContainXmlPath: function (xpath) {
                 var xml = this.actual;
                 var notText = this.isNot ? 'not ' : '';
    -            this.message = function () {
    -                return 'Expected xml \'' + et.tostring(xml) + '\' ' + notText + 'to contain elements matching \'' + xpath + '\'.';
    -            };
    -
    -            return xml.find(xpath) !== null;
    -        }    });
    +            // this.message = function () {
    +            //     return 'Expected xml \'' + et.tostring(xml) + '\' ' + notText + 'to contain elements matching \'' + xpath + '\'.';
    +            // };
    +            return {
    +                compare: function(actual, expected) {
    +                    var result = {};
    +                    result.pass = fs.existsSync(actual);
    +
    +            //return xml.find(xpath) !== null;
    +                    if(result.pass) {
    +                        result.message = "expected " + actual + " to not exist";
    +                    } else {
    +                        result.message = "expected " + actual + " to exist";
    --- End diff --
    
    update second message to 
    
    `'Expected xml \'' + et.tostring(xml) + '\' ' +' to contain elements matching \'' + expected + '\'.';`


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95501387
  
    --- Diff: cordova-lib/spec-plugman/create.spec.js ---
    @@ -35,46 +35,42 @@ describe( 'create plugin', function() {
             existsSync,
             mkdir,
             writeFileSync;
    -    function createPromise( f ) {
    -        f.then( function() { done = true; }, function(err) { done = err; } );
    -    }
    +
         beforeEach( function() {
    -        existsSync = spyOn( fs, 'existsSync' ).andReturn( false );
    -        mkdir = spyOn( shell, 'mkdir' ).andReturn( true );
    +        existsSync = spyOn( fs, 'existsSync' ).and.returnValue( false );
    +        mkdir = spyOn( shell, 'mkdir' ).and.returnValue( true );
             writeFileSync = spyOn( fs, 'writeFileSync' );
             done = false;
         });
     
    -    it( 'should be successful', function() {
    -        runs(function() {
    -            createPromise( create( 'name', 'org.plugin.id', '0.0.0', '.', [] ) );
    -        });
    -        waitsFor(function() { return done; }, 'create promise never resolved', 500);
    -        runs(function() {
    -            expect( done ).toBe( true );
    -            expect( writeFileSync.calls.length ).toEqual( 2 );
    +    it( 'Test 002 : should be successful', function(done) {
    +        create('name', 'org.plugin.id', '0.0.0', '.', [])
    +        .then(function(result) {
    +            expect( writeFileSync.calls.count() ).toEqual( 2 );
    +            done();
    +        }).fail(function err(errMsg) {
    +            done();
    --- End diff --
    
    maybe add a expect errMsg to be undefined here


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95500826
  
    --- Diff: cordova-lib/spec-cordova/plugin.spec.js ---
    @@ -216,91 +229,110 @@ describe('plugin end-to-end', function() {
                 expect(defaultPluginPreferences.REQUIRED).toBe('NO');
                 return removePlugin(org_test_defaultvariables);
            })
    -       .fail(errorHandler.errorCallback)
    -       .fin(done);
    -    });
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
    +        .fin(done);
    +    }, 30000);
     
    -    it('should successfully add a plugin when specifying CLI variables', function(done) {
    +    it('Test 006 : should successfully add a plugin when specifying CLI variables', function(done) {
             addPlugin(path.join(pluginsDir, org_test_defaultvariables), org_test_defaultvariables, {cli_variables: { REQUIRED:'yes', REQUIRED_ANDROID:'yes'}}, done)
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
             .fin(done);
    -    });
    +    }, 30000);
     
    -    it('should not check npm info when using the searchpath flag', function(done) {
    +    it('Test 007 : should not check npm info when using the searchpath flag', function(done) {
             mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin));
    -
             spyOn(registry, 'info');
    -        addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {searchpath: pluginsDir}, done)
    +        return addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {searchpath: pluginsDir}, done)
             .then(function() {
                 expect(registry.info).not.toHaveBeenCalled();
    -
    -            var fetchOptions = plugman.raw.fetch.mostRecentCall.args[2];
    -            expect(fetchOptions.searchpath).toBeDefined();
    +            var fetchOptions = plugman.raw.fetch.calls.mostRecent().args[2];
    +            expect(fetchOptions.searchpath[0]).toExist();
    +        })
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
             })
    -        .fail(errorHandler.errorCallback)
             .fin(done);
    -    });
    +    }, 30000);
     
    -    it('should not check npm info when using the noregistry flag', function(done) {
    +    it('Test 008 : should not check npm info when using the noregistry flag', function(done) {
             mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin));
     
             spyOn(registry, 'info');
             addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {noregistry:true}, done)
             .then(function() {
                 expect(registry.info).not.toHaveBeenCalled();
     
    -            var fetchOptions = plugman.raw.fetch.mostRecentCall.args[2];
    +            var fetchOptions = plugman.raw.fetch.calls.mostRecent().args[2];
                 expect(fetchOptions.noregistry).toBeTruthy();
             })
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    --- End diff --
    
    same as 155


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r93506823
  
    --- Diff: cordova-lib/spec-plugman/platforms/ios.spec.js ---
    @@ -301,14 +301,8 @@ describe('ios project handler', function() {
                 });
             });
             it('of two plugins should apply xcode file changes from both', function(){
    --- End diff --
    
    in your first comment, you had done outside the promise chain. In your second comment, you have done in the correct spot. Order shouldn't matter. 


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by audreyso <gi...@git.apache.org>.
Github user audreyso commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r92903756
  
    --- Diff: cordova-lib/spec-plugman/install.spec.js ---
    @@ -345,13 +344,14 @@ describe('install', function() {
     
             it('should not check custom engine version that is not supported for platform', function() {
                 var spy = spyOn(semver, 'satisfies').and.returnValue(true);
    -            runs(function() {
    -                installPromise( install('blackberry10', project, plugins['com.cordova.engine']) );
    -            });
    -            waitsFor(function() { return done; }, 'install promise never resolved', 200);
    -            runs(function() {
    +            install('blackberry10', project, plugins['com.cordova.engine']).then(function(done) {
    +                expect(false).toBe(true);
    +                done();
    +            },
    +            function err(errMsg) {
                     expect(spy).not.toHaveBeenCalledWith('','>=3.0.0');
    --- End diff --
    
    Thanks @stevengill ... It looks like in some of the other tests, it is going right to fail on purpose? I can rewrite the test like this 
    
    ```
            it('should not check custom engine version that is not supported for platform', function() {
                var spy = spyOn(semver, 'satisfies').and.returnValue(true);
    
                install('blackberry10', project, plugins['com.cordova.engine'])
                .then(function() {
                }).fail(function(error) {
                    expect(spy).not.toHaveBeenCalledWith('','>=3.0.0');
                }).fin (done); 
    }); ```
    
    Is this better? It still goes right to the fail.



---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cordova-lib/pull/510


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95500857
  
    --- Diff: cordova-lib/spec-cordova/plugin.spec.js ---
    @@ -216,91 +229,110 @@ describe('plugin end-to-end', function() {
                 expect(defaultPluginPreferences.REQUIRED).toBe('NO');
                 return removePlugin(org_test_defaultvariables);
            })
    -       .fail(errorHandler.errorCallback)
    -       .fin(done);
    -    });
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
    +        .fin(done);
    +    }, 30000);
     
    -    it('should successfully add a plugin when specifying CLI variables', function(done) {
    +    it('Test 006 : should successfully add a plugin when specifying CLI variables', function(done) {
             addPlugin(path.join(pluginsDir, org_test_defaultvariables), org_test_defaultvariables, {cli_variables: { REQUIRED:'yes', REQUIRED_ANDROID:'yes'}}, done)
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
             .fin(done);
    -    });
    +    }, 30000);
     
    -    it('should not check npm info when using the searchpath flag', function(done) {
    +    it('Test 007 : should not check npm info when using the searchpath flag', function(done) {
             mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin));
    -
             spyOn(registry, 'info');
    -        addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {searchpath: pluginsDir}, done)
    +        return addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {searchpath: pluginsDir}, done)
             .then(function() {
                 expect(registry.info).not.toHaveBeenCalled();
    -
    -            var fetchOptions = plugman.raw.fetch.mostRecentCall.args[2];
    -            expect(fetchOptions.searchpath).toBeDefined();
    +            var fetchOptions = plugman.raw.fetch.calls.mostRecent().args[2];
    +            expect(fetchOptions.searchpath[0]).toExist();
    +        })
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
             })
    -        .fail(errorHandler.errorCallback)
             .fin(done);
    -    });
    +    }, 30000);
     
    -    it('should not check npm info when using the noregistry flag', function(done) {
    +    it('Test 008 : should not check npm info when using the noregistry flag', function(done) {
             mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin));
     
             spyOn(registry, 'info');
             addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {noregistry:true}, done)
             .then(function() {
                 expect(registry.info).not.toHaveBeenCalled();
     
    -            var fetchOptions = plugman.raw.fetch.mostRecentCall.args[2];
    +            var fetchOptions = plugman.raw.fetch.calls.mostRecent().args[2];
                 expect(fetchOptions.noregistry).toBeTruthy();
             })
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
             .fin(done);
    -    });
    +    }, 30000);
     
    -    it('should not check npm info when fetching from a Git repository', function(done) {
    +    it('Test 009 : should not check npm info when fetching from a Git repository', function(done) {
             spyOn(registry, 'info');
             addPlugin(testGitPluginRepository, testGitPluginId, {}, done)
             .then(function() {
                 expect(registry.info).not.toHaveBeenCalled();
             })
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    +            expect(err).toBeUndefined();
    +        })
             .fin(done);
    -    });
    +    }, 30000);
     
    -    it('should select the plugin version based on npm info when fetching from npm', function(done) {
    +    it('Test 010 : should select the plugin version based on npm info when fetching from npm', function(done) {
             mockPluginFetch(npmInfoTestPlugin, path.join(pluginsDir, npmInfoTestPlugin));
     
    -        spyOn(registry, 'info').andCallThrough();
    +        spyOn(registry, 'info').and.callThrough();
             addPlugin(npmInfoTestPlugin, npmInfoTestPlugin, {}, done)
             .then(function() {
                 expect(registry.info).toHaveBeenCalled();
     
    -            var fetchTarget = plugman.raw.fetch.mostRecentCall.args[0];
    +            var fetchTarget = plugman.raw.fetch.calls.mostRecent().args[0];
                 expect(fetchTarget).toEqual(npmInfoTestPlugin + '@' + npmInfoTestPluginVersion);
             })
    -        .fail(errorHandler.errorCallback)
    +        .fail(function(err) {
    +            console.error(err);
    --- End diff --
    
    same as 155


---
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


[GitHub] cordova-lib issue #510: Fixjasmine : CB:12018 - updating tests in cordova-li...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on the issue:

    https://github.com/apache/cordova-lib/pull/510
  
    Can you also update package.json for cordova-lib and cordova-common, removing jasmine-node and adding jasmine, + updating the package.json scripts (like test) to use jasmine instead of jasmine-node


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by audreyso <gi...@git.apache.org>.
Github user audreyso commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r92480797
  
    --- Diff: cordova-lib/spec-cordova/lazy_load.spec.js ---
    @@ -34,8 +34,8 @@ var lazy_load = require('../src/cordova/lazy_load'),
     describe('lazy_load module', function() {
         var custom_path, cachePackage, fakeLazyLoad;
         beforeEach(function() {
    -        custom_path = spyOn(config, 'has_custom_path').andReturn(false);
    -        cachePackage = spyOn(npmHelper, 'cachePackage').andReturn(Q(path.join('lib', 'dir')));
    +        custom_path = spyOn(config, 'has_custom_path').and.returnValue(false);
    +        cachePackage = spyOn(npmHelper, 'cachePackage').and.returnValue(false);
    --- End diff --
    
    Ah! Thanks will fix this part!


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95502042
  
    --- Diff: cordova-lib/spec-plugman/platforms/windows.spec.js ---
    @@ -121,7 +131,8 @@ beforeEach(function () {
                 });
     
                 function validateInstalledProjects(tag, elementToInstall, xpath, supportedPlatforms) {
    -                jasmine.getEnv().currentSpec.removeAllSpies();
    +                //jasmine.getEnv().currentSpec.removeAllSpies();
    --- End diff --
    
    delete this


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r93491307
  
    --- Diff: cordova-lib/spec-plugman/platforms/windows.spec.js ---
    @@ -50,18 +50,30 @@ function copyArray(arr) {
     }
     
     beforeEach(function () {
    -    this.addMatchers({
    +    jasmine.addMatchers({
             toContainXmlPath: function (xpath) {
    --- End diff --
    
    remove xpath argument. Not needed


---
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


[GitHub] cordova-lib issue #510: Fixjasmine : CB:12018 - updating tests in cordova-li...

Posted by codecov-io <gi...@git.apache.org>.
Github user codecov-io commented on the issue:

    https://github.com/apache/cordova-lib/pull/510
  
    ## [Current coverage](https://codecov.io/gh/apache/cordova-lib/pull/510?src=pr) is 52.15% (diff: 100%)
    
    
    > No coverage report found for **master** at 84fa8e3.
    
    > Powered by [Codecov](https://codecov.io?src=pr). Last update [84fa8e3...b0e427c](https://codecov.io/gh/apache/cordova-lib/compare/84fa8e39b22252381ed4baa6868ff6f22b7a48e2...b0e427c675dc6f0d112cb91c961780b27677a8c2?src=pr)


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r95501615
  
    --- Diff: cordova-lib/spec-plugman/install.spec.js ---
    @@ -206,134 +178,136 @@ describe('install', function() {
     
         beforeEach(function() {
     
    -        exec = spyOn(child_process, 'exec').andCallFake(function(cmd, cb) {
    +        exec = spyOn(child_process, 'exec').and.callFake(function(cmd, cb) {
                 cb(false, '', '');
             });
    -        spawnSpy = spyOn(superspawn, 'spawn').andReturn(Q('3.1.0'));
    -        spyOn(fs, 'mkdirSync').andReturn(true);
    -        spyOn(shell, 'mkdir').andReturn(true);
    -        spyOn(platforms, 'copyFile').andReturn(true);
    -
    -        fetchSpy = spyOn(plugman.raw, 'fetch').andReturn( Q( plugins['com.cordova.engine'] ) );
    -        chmod = spyOn(fs, 'chmodSync').andReturn(true);
    -        spyOn(fs, 'writeFileSync').andReturn(true);
    -        cp = spyOn(shell, 'cp').andReturn(true);
    -        rm = spyOn(shell, 'rm').andReturn(true);
    +        spawnSpy = spyOn(superspawn, 'spawn').and.returnValue(Q('3.1.0'));
    +        spyOn(fs, 'mkdirSync').and.returnValue(true);
    +        spyOn(shell, 'mkdir').and.returnValue(true);
    +        spyOn(platforms, 'copyFile').and.returnValue(true);
    +
    +        fetchSpy = spyOn(plugman.raw, 'fetch').and.returnValue( Q( plugins['com.cordova.engine'] ) );
    +        chmod = spyOn(fs, 'chmodSync').and.returnValue(true);
    +        spyOn(fs, 'writeFileSync').and.returnValue(true);
    +        cp = spyOn(shell, 'cp').and.returnValue(true);
    +        rm = spyOn(shell, 'rm').and.returnValue(true);
             add_to_queue = spyOn(PlatformJson.prototype, 'addInstalledPluginToPrepareQueue');
             done = false;
         });
     
         describe('success', function() {
    -        it('should emit a results event with platform-agnostic <info>', function() {
    +        it('Test 002 : should emit a results event with platform-agnostic <info>', function() {
                 // org.test.plugins.childbrowser
                 expect(results['emit_results'][0]).toBe('No matter what platform you are installing to, this notice is very important.');
             });
    -        it('should emit a results event with platform-specific <info>', function() {
    +        it('Test 003 : should emit a results event with platform-specific <info>', function() {
                 // org.test.plugins.childbrowser
                 expect(results['emit_results'][1]).toBe('Please make sure you read this because it is very important to complete the installation of your plugin.');
             });
    -        it('should interpolate variables into <info> tags', function() {
    +        it('Test 004 : should interpolate variables into <info> tags', function() {
                 // VariableBrowser
                 expect(results['emit_results'][2]).toBe('Remember that your api key is batman!');
             });
    -        it('should call fetch if provided plugin cannot be resolved locally', function() {
    -            fetchSpy.andReturn( Q( plugins['org.test.plugins.dummyplugin'] ) );
    -            spyOn(fs, 'existsSync').andCallFake( fake['existsSync']['noPlugins'] );
    -
    -            runs(function() {
    -                installPromise(install('android', project, 'CLEANYOURSHORTS' ));
    -            });
    -            waitsFor(function() { return done; }, 'install promise never resolved', 200);
    -            runs(function() {
    -                expect(done).toBe(true);
    +        it('Test 005 : should call fetch if provided plugin cannot be resolved locally', function(done) {
    +            fetchSpy.and.returnValue( Q( plugins['org.test.plugins.dummyplugin'] ) );
    +            spyOn(fs, 'existsSync').and.callFake( fake['existsSync']['noPlugins'] );
    +            install('android', project, 'CLEANYOURSHORTS')
    +            .fail(function(err){
    +                console.log(err);
    +            })
    +            .fin(function () {
                     expect(fetchSpy).toHaveBeenCalled();
    +                done();
                 });
             });
    -        it('should call fetch and convert oldID to newID', function() {
    -            fetchSpy.andReturn( Q( plugins['org.test.plugins.dummyplugin'] ) );
    -            spyOn(fs, 'existsSync').andCallFake( fake['existsSync']['noPlugins'] );
    +
    +        it('Test 006 : should call fetch and convert oldID to newID', function(done) {
    +            fetchSpy.and.returnValue( Q( plugins['org.test.plugins.dummyplugin'] ) );
    +            spyOn(fs, 'existsSync').and.callFake( fake['existsSync']['noPlugins'] );
                 var emit = spyOn(events, 'emit');
    -            runs(function() {
    -                installPromise(install('android', project, 'org.apache.cordova.device' ));
    -            });
    -            waitsFor(function() { return done; }, 'install promise never resolved', 200);
    -            runs(function() {
    -                expect(emit.calls[0].args[1]).toBe('Notice: org.apache.cordova.device has been automatically converted to cordova-plugin-device and fetched from npm. This is due to our old plugins registry shutting down.');
    -                expect(done).toBe(true);
    +            install('android', project, 'org.apache.cordova.device' )
    +            .then(function(res) {
    +                return true;
    +            })
    +            .fail(function(err){
    +                console.log(err);
    --- End diff --
    
    same as 216


---
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


[GitHub] cordova-lib pull request #510: Fixjasmine : CB:12018 - updating tests in cor...

Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:

    https://github.com/apache/cordova-lib/pull/510#discussion_r93492375
  
    --- Diff: cordova-lib/spec-plugman/platforms/wp8.spec.js ---
    @@ -113,22 +113,21 @@ describe('wp8 project handler', function() {
                     fs.writeFileSync(target, 'some bs', 'utf-8');
                     expect(function() {
                         wp8['source-file'].install(source[0], dummyplugin, temp, dummy_id, null, proj_files);
    -                }).toThrow('"' + target + '" already exists!');
    +                }).toThrow(new Error ('"' + target + '" already exists!'));
                 });
             });
             describe('of <config-changes> elements', function() {
                 beforeEach(function() {
                     shell.cp('-rf', path.join(wp8_project, '*'), temp);
                 });
    -            it('should process and pass the after parameter to graftXML', function () {
    -                var graftXML = spyOn(xml_helpers, 'graftXML').andCallThrough();
    -
    -                runs(function () { installPromise(install('wp8', temp, dummyplugin, plugins_dir, {})); });
    -                waitsFor(function () { return done; }, 'install promise never resolved', 500);
    -                runs(function () {
    +            it('should process and pass the after parameter to graftXML', function (done) {
    +                var graftXML = spyOn(xml_helpers, 'graftXML').and.callThrough();
    +                return install('wp8', temp, dummyplugin, plugins_dir, {})
    +                .then(function(result) {
    --- End diff --
    
    don't need the result argument here since you don't use it


---
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