[GitHub] cordova-paramedic pull request #18: Appium updates for Paramedic
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-paramedic/pull/18#discussion_r92491112
--- Diff: lib/appium/AppiumRunner.js ---
@@ -102,6 +102,24 @@ function getConfigPath(appPath) {
return path.join(appPath, 'config.xml');
}
+function installAppiumServer() {
+/*
+ * We install appium in this counter-intuitive way at runtime in order
to
+ * not trigger massive dependency downloads via `npm install` for
autotest
+ * execution. Installing appium only happens in the local-running
case, and
+ * thus is not needed in our CI system. By using this method, we save
the CI
+ * a lot of time.
+ * ALTERNATIVE: what if paramedic dependencies were installed via `npm
install
+ * --production`, and appium was stuffed into `devDependencies`. That
should
+ * also prevent this problem?
--- End diff --
@purplecabbage Can you elaborate how either of the current approach or
putting appium into devDependencies sacrifices the individual plugin developer?
I don't follow.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-paramedic pull request #18: Appium updates for Paramedic
Github user purplecabbage commented on a diff in the pull request:
https://github.com/apache/cordova-paramedic/pull/18#discussion_r92469947
--- Diff: lib/appium/AppiumRunner.js ---
@@ -102,6 +102,24 @@ function getConfigPath(appPath) {
return path.join(appPath, 'config.xml');
}
+function installAppiumServer() {
+/*
+ * We install appium in this counter-intuitive way at runtime in order
to
+ * not trigger massive dependency downloads via `npm install` for
autotest
+ * execution. Installing appium only happens in the local-running
case, and
+ * thus is not needed in our CI system. By using this method, we save
the CI
+ * a lot of time.
+ * ALTERNATIVE: what if paramedic dependencies were installed via `npm
install
+ * --production`, and appium was stuffed into `devDependencies`. That
should
+ * also prevent this problem?
--- End diff --
I agree with @alsorokin here. Can we avoid sacrificing the individual
plugin developer for the sake of the CI server?
This seems like it's maybe a case where we could/should use a *-connect
approach and have another module that provides the connection to appium. Or
maybe we use feature detection to poke for the appium module and use it if it's
present. ... not sure
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-paramedic pull request #18: Appium updates for Paramedic
Github user alsorokin commented on a diff in the pull request:
https://github.com/apache/cordova-paramedic/pull/18#discussion_r92348958
--- Diff: lib/appium/AppiumRunner.js ---
@@ -102,6 +102,24 @@ function getConfigPath(appPath) {
return path.join(appPath, 'config.xml');
}
+function installAppiumServer() {
+/*
+ * We install appium in this counter-intuitive way at runtime in order
to
+ * not trigger massive dependency downloads via `npm install` for
autotest
+ * execution. Installing appium only happens in the local-running
case, and
+ * thus is not needed in our CI system. By using this method, we save
the CI
+ * a lot of time.
+ * ALTERNATIVE: what if paramedic dependencies were installed via `npm
install
+ * --production`, and appium was stuffed into `devDependencies`. That
should
+ * also prevent this problem?
--- End diff --
That is certainly a viable option, but I'm still reluctant to do so because
paramedic is used not only in CI, but also by people who want to test things
quickly and they would certainly run `npm i` and not `npm i --production` and
end up with extra 5 minutes of installation and 250MB of dependencies which are
not used in most cases. Not to mention various npm installation issues...
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-paramedic pull request #18: Appium updates for Paramedic
Github user alsorokin commented on a diff in the pull request:
https://github.com/apache/cordova-paramedic/pull/18#discussion_r92190252
--- Diff: package.json ---
@@ -51,7 +51,7 @@
"wd": "^0.4.0"
},
"devDependencies": {
-"jasmine-node": "~1",
-"jshint": "^2.6.0"
+"jshint": "^2.6.0",
+"appium": "1.5.3"
--- End diff --
We call this method only when running Appium tests locally, so we save time
in all other runs - we don't have to install pretty heavy `appium` module when
running on Sauce Labs, for example.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-paramedic pull request #18: Appium updates for Paramedic
Github user alsorokin commented on a diff in the pull request:
https://github.com/apache/cordova-paramedic/pull/18#discussion_r92122133
--- Diff: lib/paramedic.js ---
@@ -271,9 +271,13 @@ ParamedicRunner.prototype.runLocalTests = function () {
})
.then(function(command) {
self.setPermissions();
-logger.normal('cordova-paramedic: running command ' + command);
-return execPromise(command);
+if (!self.config.runMainTests()) {
--- End diff --
My bad, it actually works because the Android emulator gets started by
`ParamedicTargetChooser`.
Then the aforementioned comment should also be deleted.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-paramedic pull request #18: Appium updates for Paramedic
Github user alsorokin commented on a diff in the pull request:
https://github.com/apache/cordova-paramedic/pull/18#discussion_r92111761
--- Diff: package.json ---
@@ -51,7 +51,7 @@
"wd": "^0.4.0"
},
"devDependencies": {
-"jasmine-node": "~1",
-"jshint": "^2.6.0"
+"jshint": "^2.6.0",
+"appium": "1.5.3"
--- End diff --
Adding Appium to dependencies would drastically increase paramedic install
time for every build on CI, while only a small subset of them actually needs it.
I think we can still install this particular version via executing `npm i
appium@1.5.3` in `installAppiumServer()` function, aren't we?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-paramedic pull request #18: Appium updates for Paramedic
Github user alsorokin commented on a diff in the pull request:
https://github.com/apache/cordova-paramedic/pull/18#discussion_r92113708
--- Diff: lib/appium/AppiumRunner.js ---
@@ -419,8 +411,9 @@ AppiumRunner.prototype.prepareApp = function () {
var deviceString = self.options.device ? ' --device' : '';
var buildCommand = 'cordova build ' + self.options.platform +
deviceString;
+// TODO: is removing medic.json still needed?
--- End diff --
Yes, this is still needed because we use this file to pass local medic
server URI to paramedic plugin so it could report test results back to
paramedic. We remove it before the Appium run so the test framework plugin
won't start plugin tests right away.
So, basically this is needed to run Appium tests without running other
plugin tests simultaneously.
This probably should've been mentioned by me in this 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
[GitHub] cordova-paramedic pull request #18: Appium updates for Paramedic
Github user alsorokin commented on a diff in the pull request:
https://github.com/apache/cordova-paramedic/pull/18#discussion_r92112118
--- Diff: lib/paramedic.js ---
@@ -271,9 +271,13 @@ ParamedicRunner.prototype.runLocalTests = function () {
})
.then(function(command) {
self.setPermissions();
-logger.normal('cordova-paramedic: running command ' + command);
-return execPromise(command);
+if (!self.config.runMainTests()) {
--- End diff --
This change would break local Android Appium run, I believe. See the
comment on line 283.
We need to boot up an Android emulator for Appium to run tests on and
skipping this command would skip starting the emulator.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org
