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 2016/02/22 17:10:46 UTC
[GitHub] cordova-plugin-camera pull request: CB-10639 Appium tests: Added s...
GitHub user alsorokin opened a pull request:
https://github.com/apache/cordova-plugin-camera/pull/177
CB-10639 Appium tests: Added some timeouts,
Taking a screenshot on failure,
Retry taking a picture up to 3 times,
Try to restart the Appium session if it's lost
https://issues.apache.org/jira/browse/CB-10639
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-plugin-camera CB-10639
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-plugin-camera/pull/177.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 #177
----
commit 741a9ff1e988acd8a9ad53018f06f218ed34a00c
Author: Alexander Sorokin <al...@akvelon.com>
Date: 2016-02-22T15:37:41Z
CB-10639 Appium tests: Added some timeouts,
Taking a screenshot on failure,
Retry taking a picture up to 3 times,
Try to restart the Appium session if it's lost
----
---
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-plugin-camera pull request: CB-10639 Appium tests: Added s...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-camera/pull/177#discussion_r53877650
--- Diff: appium-tests/android/android.spec.js ---
@@ -1,28 +1,34 @@
-/*jslint node: true, plusplus: true */
+/*jshint node: true */
/*global beforeEach, afterEach */
/*global describe, it, xit, expect, jasmine */
-'use strict';
// these tests are meant to be executed by Cordova Medic Appium runner
// you can find it here: https://github.com/apache/cordova-medic/
// it is not necessary to do a full CI setup to run these tests
// just run "node cordova-medic/medic/medic.js appium --platform android --plugins cordova-plugin-camera"
+'use strict';
+
var wdHelper = require('../helpers/wdHelper');
var wd = wdHelper.getWD();
var cameraConstants = require('../../www/CameraConstants');
var cameraHelper = require('../helpers/cameraHelper');
+var screenshotHelper = require('../helpers/screenshotHelper');
describe('Camera tests Android.', function () {
+
+ var STARTING_MESSAGE = 'Ready for action!',
+ RETRY_COUNT = 3, // how many times to retry taking a picture before failing
+ MINUTE = 60 * 1000;
+
var driver,
- startingMessage = 'Ready for action!',
// the name of webview context, it will be changed to match needed context if there are named ones:
webviewContext = 'WEBVIEW',
- // this indicates if device library has test picture:
+ // this indicates if device library has the test picture:
isTestPictureSaved = false,
- // this indecates that there was critical error and tests cannot continue:
+ // this indecates that there was a critical error and tests cannot continue:
--- End diff --
"indecates" -> "indicates"
---
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-plugin-camera pull request: CB-10639 Appium tests: Added s...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-plugin-camera/pull/177
---
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-plugin-camera pull request: CB-10639 Appium tests: Added s...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-camera/pull/177#discussion_r53735364
--- Diff: appium-tests/android/android.spec.js ---
@@ -229,7 +255,8 @@ describe('Camera tests Android.', function () {
pass: false,
message: msg
};
- if (msg.indexOf('Error response status: 6') >= 0) {
+ if (msg.indexOf('Error response status: 6') >= 0 ||
+ msg.indexOf('Error response status: 7') >= 0) {
--- End diff --
Please comment the significance of these strings.
---
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-plugin-camera pull request: CB-10639 Appium tests: Added s...
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-plugin-camera/pull/177#discussion_r53833681
--- Diff: appium-tests/android/android.spec.js ---
@@ -1,28 +1,34 @@
-/*jslint node: true, plusplus: true */
+/*jshint node: true */
/*global beforeEach, afterEach */
--- End diff --
Minor nit: this might be easier to use something like `/*jshint node:true, jasmine: true*/` to avoid declaring all Jasmine globals
---
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-plugin-camera pull request: CB-10639 Appium tests: Added s...
Posted by alsorokin <gi...@git.apache.org>.
Github user alsorokin commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-camera/pull/177#discussion_r53848875
--- Diff: appium-tests/android/android.spec.js ---
@@ -1,28 +1,34 @@
-/*jslint node: true, plusplus: true */
+/*jshint node: true */
/*global beforeEach, afterEach */
/*global describe, it, xit, expect, jasmine */
-'use strict';
// these tests are meant to be executed by Cordova Medic Appium runner
// you can find it here: https://github.com/apache/cordova-medic/
// it is not necessary to do a full CI setup to run these tests
// just run "node cordova-medic/medic/medic.js appium --platform android --plugins cordova-plugin-camera"
+'use strict';
+
var wdHelper = require('../helpers/wdHelper');
var wd = wdHelper.getWD();
var cameraConstants = require('../../www/CameraConstants');
var cameraHelper = require('../helpers/cameraHelper');
+var screenshotHelper = require('../helpers/screenshotHelper');
describe('Camera tests Android.', function () {
+
+ var STARTING_MESSAGE = 'Ready for action!',
+ RETRY_COUNT = 3, // how many times to retry taking a picture before failing
+ MINUTE = 60 * 1000;
+
var driver,
- startingMessage = 'Ready for action!',
// the name of webview context, it will be changed to match needed context if there are named ones:
webviewContext = 'WEBVIEW',
- // this indicates if device library has test picture:
+ // this indicates if device library has the test picture:
isTestPictureSaved = false,
- // this indecates that there was critical error and tests cannot continue:
+ // this indecates that there was a critical error and tests cannot continue:
stopFlag = false,
- // we need to know the screen width and height to properly click on the first image in the gallery
+ // we need to know the screen width and height to properly click on an image in the gallery
screenWidth = 360,
screenHeight = 567;
--- End diff --
No, these values can change when we determining the real device resolution.
These are just default values.
---
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-plugin-camera pull request: CB-10639 Appium tests: Added s...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-camera/pull/177#discussion_r53878681
--- Diff: appium-tests/android/android.spec.js ---
@@ -1,28 +1,34 @@
-/*jslint node: true, plusplus: true */
+/*jshint node: true */
/*global beforeEach, afterEach */
/*global describe, it, xit, expect, jasmine */
-'use strict';
// these tests are meant to be executed by Cordova Medic Appium runner
// you can find it here: https://github.com/apache/cordova-medic/
// it is not necessary to do a full CI setup to run these tests
// just run "node cordova-medic/medic/medic.js appium --platform android --plugins cordova-plugin-camera"
+'use strict';
+
var wdHelper = require('../helpers/wdHelper');
var wd = wdHelper.getWD();
var cameraConstants = require('../../www/CameraConstants');
var cameraHelper = require('../helpers/cameraHelper');
+var screenshotHelper = require('../helpers/screenshotHelper');
describe('Camera tests Android.', function () {
+
+ var STARTING_MESSAGE = 'Ready for action!',
+ RETRY_COUNT = 3, // how many times to retry taking a picture before failing
+ MINUTE = 60 * 1000;
+
var driver,
- startingMessage = 'Ready for action!',
// the name of webview context, it will be changed to match needed context if there are named ones:
webviewContext = 'WEBVIEW',
- // this indicates if device library has test picture:
+ // this indicates if device library has the test picture:
isTestPictureSaved = false,
- // this indecates that there was critical error and tests cannot continue:
+ // this indecates that there was a critical error and tests cannot continue:
stopFlag = false,
- // we need to know the screen width and height to properly click on the first image in the gallery
+ // we need to know the screen width and height to properly click on an image in the gallery
screenWidth = 360,
screenHeight = 567;
--- End diff --
Please then set the defaults to be constants at the top just so we don't have seemingly magic values in the 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-plugin-camera pull request: CB-10639 Appium tests: Added s...
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-plugin-camera/pull/177#discussion_r53838196
--- Diff: appium-tests/android/android.spec.js ---
@@ -295,13 +318,35 @@ describe('Camera tests Android.', function () {
.then(function () {
return driver; // no-op
}, function (error) {
- expect(true).toFailWithMessage(error);
+ if (error.message.indexOf('Error response status: 6') >= 0) {
+ // the session has expired but we can fix this!
+ console.log('The session has expired. Trying to start a new one...');
+ return getDriver();
+ } else {
+ expect(true).toFailWithMessage(error);
+ }
})
- .execute('document.getElementById("info").innerHTML = "' + startingMessage + '";')
+ .execute('document.getElementById("info").innerHTML = "' + STARTING_MESSAGE + '";')
.finally(done);
}
done();
- }, 600000);
+ }, 3 * MINUTE);
+
+ afterEach(function (done) {
+ // recreate the session if there was a critical error in the spec
+ if (stopFlag) {
+ return driver
--- End diff --
This is not obvious that you're `return`'ing a promise here just to avoid second `done` call at the end of `afterEach`
I'd recommend you to reverse `if` condition to enclose the shortest code path and reduce nesting, and to explicitly do a return after `done`. Something like:
```javascript
if (!stopFlag) {
done();
return;
}
// promise chain 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-plugin-camera pull request: CB-10639 Appium tests: Added s...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on the pull request:
https://github.com/apache/cordova-plugin-camera/pull/177#issuecomment-187991977
Thanks for fixing some of the grammar errors in the comments, btw!
---
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-plugin-camera pull request: CB-10639 Appium tests: Added s...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-camera/pull/177#discussion_r53735234
--- Diff: appium-tests/android/android.spec.js ---
@@ -1,28 +1,34 @@
-/*jslint node: true, plusplus: true */
+/*jshint node: true */
/*global beforeEach, afterEach */
/*global describe, it, xit, expect, jasmine */
-'use strict';
// these tests are meant to be executed by Cordova Medic Appium runner
// you can find it here: https://github.com/apache/cordova-medic/
// it is not necessary to do a full CI setup to run these tests
// just run "node cordova-medic/medic/medic.js appium --platform android --plugins cordova-plugin-camera"
+'use strict';
+
var wdHelper = require('../helpers/wdHelper');
var wd = wdHelper.getWD();
var cameraConstants = require('../../www/CameraConstants');
var cameraHelper = require('../helpers/cameraHelper');
+var screenshotHelper = require('../helpers/screenshotHelper');
describe('Camera tests Android.', function () {
+
+ var STARTING_MESSAGE = 'Ready for action!',
+ RETRY_COUNT = 3, // how many times to retry taking a picture before failing
+ MINUTE = 60 * 1000;
+
var driver,
- startingMessage = 'Ready for action!',
// the name of webview context, it will be changed to match needed context if there are named ones:
webviewContext = 'WEBVIEW',
- // this indicates if device library has test picture:
+ // this indicates if device library has the test picture:
isTestPictureSaved = false,
- // this indecates that there was critical error and tests cannot continue:
+ // this indecates that there was a critical error and tests cannot continue:
stopFlag = false,
- // we need to know the screen width and height to properly click on the first image in the gallery
+ // we need to know the screen width and height to properly click on an image in the gallery
screenWidth = 360,
screenHeight = 567;
--- End diff --
Should these guys also be constants?
---
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-plugin-camera pull request: CB-10639 Appium tests: Added s...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-camera/pull/177#discussion_r53735288
--- Diff: appium-tests/ios/ios.spec.js ---
@@ -8,8 +14,12 @@ var wd = wdHelper.getWD();
var isDevice = global.DEVICE;
var cameraConstants = require('../../www/CameraConstants');
var cameraHelper = require('../helpers/cameraHelper');
+var screenshotHelper = require('../helpers/screenshotHelper');
describe('Camera tests iOS.', function () {
+
+ var MINUTE = 60 * 1000;
--- End diff --
Please put this at the top of the file, outside of function scope.
---
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-plugin-camera pull request: CB-10639 Appium tests: Added s...
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-camera/pull/177#discussion_r53735222
--- Diff: appium-tests/android/android.spec.js ---
@@ -1,28 +1,34 @@
-/*jslint node: true, plusplus: true */
+/*jshint node: true */
/*global beforeEach, afterEach */
/*global describe, it, xit, expect, jasmine */
-'use strict';
// these tests are meant to be executed by Cordova Medic Appium runner
// you can find it here: https://github.com/apache/cordova-medic/
// it is not necessary to do a full CI setup to run these tests
// just run "node cordova-medic/medic/medic.js appium --platform android --plugins cordova-plugin-camera"
+'use strict';
+
var wdHelper = require('../helpers/wdHelper');
var wd = wdHelper.getWD();
var cameraConstants = require('../../www/CameraConstants');
var cameraHelper = require('../helpers/cameraHelper');
+var screenshotHelper = require('../helpers/screenshotHelper');
describe('Camera tests Android.', function () {
+
+ var STARTING_MESSAGE = 'Ready for action!',
+ RETRY_COUNT = 3, // how many times to retry taking a picture before failing
+ MINUTE = 60 * 1000;
--- End diff --
Please put these outside of function scope. Nitpick: please have one declaration per line.
---
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