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/12 17:37:53 UTC
[GitHub] cordova-cli pull request #265: CB-12018 : updated tests to function with jas...
GitHub user audreyso opened a pull request:
https://github.com/apache/cordova-cli/pull/265
CB-12018 : updated tests to function with 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?
Updated tests to function with jasmine instead of jasmine-node
### 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.
\u2026node
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/audreyso/cordova-cli CB-12018
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-cli/pull/265.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 #265
----
commit a0e80aa05cce90399160f6a6555389f83c746d87
Author: audreyso <au...@adobe.com>
Date: 2016-12-10T01:11:28Z
CB-12018 : updated tests to function with 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-cli pull request #265: CB-12018 : updated tests to function with jas...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/265#discussion_r95522000
--- Diff: spec/cli.spec.js ---
@@ -55,76 +55,76 @@ describe("cordova cli", function () {
});
describe("options", function () {
- describe("version", function () {
- var version = require("../package").version;
+ describe("version", function () {
--- End diff --
nit: could you please use 4 spaces for indentation to be consistent with the rest of the tests/code?
---
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-cli issue #265: CB-12018 : updated tests to function with jasmine in...
Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on the issue:
https://github.com/apache/cordova-cli/pull/265
@vladimir-kotikov hey! You make a good point about it being annoying to add tests later and have to update all the numbers. And I agree the stack trace is more useful. We ran into issues where the stack trace wasn't useful (especially with Q errors). That is why Audrey started labeling tests.
I'm not too concerned with the numbers staying in order. If it does become annoying, we will go remove it.
Appreciate your feedback :)
---
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-cli pull request #265: CB-12018 : updated tests to function with jas...
Posted by audreyso <gi...@git.apache.org>.
Github user audreyso commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/265#discussion_r95645698
--- Diff: spec/cli.spec.js ---
@@ -134,11 +134,11 @@ describe("cordova cli", function () {
describe("create", function () {
beforeEach(function () {
- spyOn(cordova.raw, "create").andReturn(Q());
- spyOn(cordova_lib, "CordovaError");
+ spyOn(cordova.raw, "create").and.returnValue(Q());
+ // spyOn(cordova_lib, "CordovaError");
--- End diff --
yes @stevengill ... it gives this error
``` Error: <spyOn> : CordovaError is not declared writable or has no setter
Usage: spyOn(<object>, <methodName>) ```
Is this line still needed? I tried using this to fix it, but that did not work.
``` Object.defineProperty(cordova_lib, "CordovaError", { writable: true }); ```
I've seen some people fix it like this too, but not sure if this is correct.
``` var descriptor = Object.getOwnPropertyDescriptor(obj, methodName);
if (!(descriptor.writable || descriptor.set)) {
throw new Error(methodName + ' is not declared writable or has no setter');
} ```
---
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-cli pull request #265: CB-12018 : updated tests to function with jas...
Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/265#discussion_r95493028
--- Diff: spec/cli.spec.js ---
@@ -55,76 +55,76 @@ describe("cordova cli", function () {
});
describe("options", function () {
- describe("version", function () {
- var version = require("../package").version;
+ describe("version", function () {
+ var version = require("../package").version;
- beforeEach(function () {
- });
-
- it("will spit out the version with -v", function (done) {
- cli(["node", "cordova", "-v"], function() {
- expect(logger.results.mostRecentCall.args[0]).toMatch(version);
- done();
- });
- });
-
- it("will spit out the version with --version", function (done) {
- cli(["node", "cordova", "--version"], function () {
- expect(logger.results.mostRecentCall.args[0]).toMatch(version);
- done()
- });
- });
+ beforeEach(function () {
--- End diff --
can probably delete this beforeEach
---
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-cli issue #265: CB-12018 : updated tests to function with jasmine in...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the issue:
https://github.com/apache/cordova-cli/pull/265
Yeah, not a big deal and probably not worth renaming all test cases back now, It just reminded me file transfer tests where we have 'spec.11' then `spec.9` then `spec.10`, etc. :)
---
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-cli pull request #265: CB-12018 : updated tests to function with jas...
Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/265#discussion_r95493138
--- Diff: spec/cli.spec.js ---
@@ -134,11 +134,11 @@ describe("cordova cli", function () {
describe("create", function () {
beforeEach(function () {
- spyOn(cordova.raw, "create").andReturn(Q());
- spyOn(cordova_lib, "CordovaError");
+ spyOn(cordova.raw, "create").and.returnValue(Q());
+ // spyOn(cordova_lib, "CordovaError");
--- End diff --
do you remember why you commented this out?
---
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-cli pull request #265: CB-12018 : updated tests to function with jas...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-cli/pull/265
---
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-cli pull request #265: CB-12018 : updated tests to function with jas...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/265#discussion_r92010506
--- Diff: package.json ---
@@ -31,6 +31,7 @@
"cordova-common": "1.5.x",
"cordova-lib": "6.4.0",
"insight": "~0.8.2",
+ "jasmine": "^2.5.2",
--- End diff --
Shouldn't this be a dev dependency?
---
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-cli pull request #265: CB-12018 : updated tests to function with jas...
Posted by audreyso <gi...@git.apache.org>.
Github user audreyso commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/265#discussion_r95635545
--- Diff: package.json ---
@@ -11,8 +11,8 @@
"cordova": "./bin/cordova"
},
"scripts": {
- "test": "node node_modules/jasmine-node/bin/jasmine-node --captureExceptions --color spec",
- "cover": "node node_modules/istanbul/lib/cli.js cover --root src --print detail node_modules/jasmine-node/bin/jasmine-node -- spec"
+ "test": "jasmine --captureExceptions --color",
+ "cover": "jasmine"
--- End diff --
Thank you! Updated to ... ``` "cover": "istanbul cover --root src --print detail jasmine" ```
---
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-cli pull request #265: CB-12018 : updated tests to function with jas...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/265#discussion_r95521991
--- Diff: package.json ---
@@ -11,8 +11,8 @@
"cordova": "./bin/cordova"
},
"scripts": {
- "test": "node node_modules/jasmine-node/bin/jasmine-node --captureExceptions --color spec",
- "cover": "node node_modules/istanbul/lib/cli.js cover --root src --print detail node_modules/jasmine-node/bin/jasmine-node -- spec"
+ "test": "jasmine --captureExceptions --color",
+ "cover": "jasmine"
--- End diff --
Does Jasmine generate coverage reports? Shouldn't this line be the same as [in apache/cordova-lib#510](https://github.com/apache/cordova-lib/pull/510/files#diff-cc6b3b84bb015909917e7c201a31f65aR23)
---
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-cli pull request #265: CB-12018 : updated tests to function with jas...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on a diff in the pull request:
https://github.com/apache/cordova-cli/pull/265#discussion_r92009821
--- Diff: package.json ---
@@ -11,7 +11,7 @@
"cordova": "./bin/cordova"
},
"scripts": {
- "test": "node node_modules/jasmine-node/bin/jasmine-node --captureExceptions --color spec",
+ "test": "node node_modules/jasmine/bin/jasmine --captureExceptions --color spec",
"cover": "node node_modules/istanbul/lib/cli.js cover --root src --print detail node_modules/jasmine-node/bin/jasmine-node -- spec"
--- End diff --
@audreyso, looks like you'll also need to update `cover` command to use jasmine inatead 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