You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by alsorokin <gi...@git.apache.org> on 2015/10/28 16:06:19 UTC

[GitHub] cordova-lib pull request: CB-9872 Fixed save.spec.11 failure

GitHub user alsorokin opened a pull request:

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

    CB-9872 Fixed save.spec.11 failure

    This is a fix for https://issues.apache.org/jira/browse/CB-9872

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

    $ git pull https://github.com/MSOpenTech/cordova-lib CB-9872

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

    https://github.com/apache/cordova-lib/pull/332.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 #332
    
----
commit 29fb069903972d8c62b3f54b25230984547c2fe2
Author: Alexander Sorokin <al...@akvelon.com>
Date:   2015-10-28T15:02:07Z

    CB-9872 Fixed save.spec.11 failure

----


---
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: CB-9872 Fixed save.spec.11 failure

Posted by alsorokin <gi...@git.apache.org>.
Github user alsorokin commented on the pull request:

    https://github.com/apache/cordova-lib/pull/332#issuecomment-152015056
  
    We were using master version of cordova-android platform and after recent changes which introduce new Api, mocks in these tests became insufficient. So basically, we now use android version 4.0.0, 4.0.1 and 4.1.0 for these tests instead of the master version.


---
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: CB-9872 Fixed save.spec.11 failure

Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on the pull request:

    https://github.com/apache/cordova-lib/pull/332#issuecomment-151918673
  
    So Alex - what was the underlying problem?


---
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: CB-9872 Fixed save.spec.11 failure

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

    https://github.com/apache/cordova-lib/pull/332#issuecomment-152255889
  
    Merge! That way I can do the tools release.


---
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: CB-9872 Fixed save.spec.11 failure

Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on the pull request:

    https://github.com/apache/cordova-lib/pull/332#issuecomment-152242161
  
    :+1:


---
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: CB-9872 Fixed save.spec.11 failure

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

    https://github.com/apache/cordova-lib/pull/332#discussion_r43327317
  
    --- Diff: cordova-lib/spec-cordova/save.spec.js ---
    @@ -244,19 +307,26 @@ describe('(save flag)', function () {
                     console.log(err);
                     expect(false).toBe(true);
                     done();
    +            }).finally(function () {
    +                revertDownloadPlatform();
                 });
    -        }, timeout);
    +        }, TIMEOUT);
     
             it('spec.11 should update spec with git url when updating using git url', function (done) {
                 helpers.setEngineSpec(appPath, platformName, platformVersionNew);
    -            platform('add', platformName + '@' + platformVersionNew)
    +            mockDownloadPlatform(platformLocalPathOld, platformVersionOld);
    +
    +            platform('add', platformName + '@' + platformVersionOld)
                 .then(function () {
    +                revertDownloadPlatform();
                     var fsExistsSync = fs.existsSync.bind(fs);
                     spyOn(fs, 'existsSync').andCallFake(function (somePath) {
                         return (somePath === path.join(appPath, 'platforms', platformName)) || fsExistsSync(somePath);
                     });
    +                mockDownloadPlatform(platformLocalPathNew, platformVersionNew);
    --- End diff --
    
    I believe I'll be able to catch platform update issues running my usual manual test cases which I do on every platform release.


---
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: CB-9872 Fixed save.spec.11 failure

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

    https://github.com/apache/cordova-lib/pull/332#issuecomment-152022199
  
    I don't like the idea of testing 4.1.0 instead of master. Is this something we will have to update everytime we release a newer version? 
    
    What is it about master that broke the tests?


---
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: CB-9872 Fixed save.spec.11 failure

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

    https://github.com/apache/cordova-lib/pull/332#issuecomment-152079484
  
    Sounds good.


---
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: CB-9872 Fixed save.spec.11 failure

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

    https://github.com/apache/cordova-lib/pull/332#discussion_r43331295
  
    --- Diff: cordova-lib/spec-cordova/save.spec.js ---
    @@ -244,19 +307,26 @@ describe('(save flag)', function () {
                     console.log(err);
                     expect(false).toBe(true);
                     done();
    +            }).finally(function () {
    +                revertDownloadPlatform();
                 });
    -        }, timeout);
    +        }, TIMEOUT);
     
             it('spec.11 should update spec with git url when updating using git url', function (done) {
                 helpers.setEngineSpec(appPath, platformName, platformVersionNew);
    -            platform('add', platformName + '@' + platformVersionNew)
    +            mockDownloadPlatform(platformLocalPathOld, platformVersionOld);
    +
    +            platform('add', platformName + '@' + platformVersionOld)
                 .then(function () {
    +                revertDownloadPlatform();
                     var fsExistsSync = fs.existsSync.bind(fs);
                     spyOn(fs, 'existsSync').andCallFake(function (somePath) {
                         return (somePath === path.join(appPath, 'platforms', platformName)) || fsExistsSync(somePath);
                     });
    +                mockDownloadPlatform(platformLocalPathNew, platformVersionNew);
    --- End diff --
    
    I think we want to have tests for these scenarios so we can verify things aren't being broken at PR/commit time rather than waiting until we're about to do a release.


---
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: CB-9872 Fixed save.spec.11 failure

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

    https://github.com/apache/cordova-lib/pull/332#discussion_r43292467
  
    --- Diff: cordova-lib/spec-cordova/save.spec.js ---
    @@ -244,19 +307,26 @@ describe('(save flag)', function () {
                     console.log(err);
                     expect(false).toBe(true);
                     done();
    +            }).finally(function () {
    +                revertDownloadPlatform();
                 });
    -        }, timeout);
    +        }, TIMEOUT);
     
             it('spec.11 should update spec with git url when updating using git url', function (done) {
                 helpers.setEngineSpec(appPath, platformName, platformVersionNew);
    -            platform('add', platformName + '@' + platformVersionNew)
    +            mockDownloadPlatform(platformLocalPathOld, platformVersionOld);
    +
    +            platform('add', platformName + '@' + platformVersionOld)
                 .then(function () {
    +                revertDownloadPlatform();
                     var fsExistsSync = fs.existsSync.bind(fs);
                     spyOn(fs, 'existsSync').andCallFake(function (somePath) {
                         return (somePath === path.join(appPath, 'platforms', platformName)) || fsExistsSync(somePath);
                     });
    +                mockDownloadPlatform(platformLocalPathNew, platformVersionNew);
    --- End diff --
    
    Using edge for this test has caught 2 `cordova-android` issues in the last week or so. While ideally we don't want `cordova-lib` tests failing because of `cordova-android` issues, should we continue using edge in these tests until we've added `update` tests to `cordova-android`?


---
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: CB-9872 Fixed save.spec.11 failure

Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the pull request:

    https://github.com/apache/cordova-lib/pull/332#issuecomment-152167831
  
    We've scheduled adding e2e `update` tests for android and refactoring these tests to abstract from real platforms as two next high-pri tasks, so for now I propose to merge this in as temporary solution to unblock release and make tests passing.
    
    Thoughts?


---
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: CB-9872 Fixed save.spec.11 failure

Posted by TimBarham <gi...@git.apache.org>.
Github user TimBarham commented on the pull request:

    https://github.com/apache/cordova-lib/pull/332#issuecomment-152024899
  
    @stevengill  - ultimately, the point of these tests is to test the `--save` feature, so going forward I don't think we want them to be at risk of being broken by platform changes in master, as long as we add tests to the platforms that cover the e2e scenarios that these tests were "accidentally" covering.


---
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: CB-9872 Fixed save.spec.11 failure

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

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


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