-
-
Notifications
You must be signed in to change notification settings - Fork 424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add mobile endpoints to run XCTest #1205
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to add commands in https://github.com/appium/appium/blob/master/commands-yml/commands/mobile-command.yml after this merge :)
Changes pushed. |
More changes pushed. |
lib/commands/xctest.js
Outdated
* @property {string} testName Name of the test (e.g.: 'XCTesterAppUITests - XCTesterAppUITests.XCTesterAppUITests/testExample') | ||
* @property {boolean} passed Did the tests pass? | ||
* @property {boolean} crashed Did the tests crash? | ||
* @property {number} duration How long did the tests take (in seconds) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these float seconds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, but a JS number can be an integer or a float.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's more about the downstream API. Usually Apple stuff uses NSTimeInterval type to describe duration and this is represented in float seconds, which means that the precision is fine for milliseconds as well and digits after the decimal point are not ignored.
lib/commands/xctest.js
Outdated
* @property {boolean} crashed Did the tests crash? | ||
* @property {number} duration How long did the tests take (in seconds) | ||
* @property {string} failureMessage Failure message (if applicable) | ||
* @property {number} location |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is location
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was no documentation on it, so not sure.
lib/commands/xctest.js
Outdated
/** | ||
* @typedef {Object} RunXCUITestOptions | ||
* | ||
* @property {string!} testRunnerBundleId Test app bundle (e.g.: 'io.appium.XCTesterAppUITests.xctrunner') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the exclamation mark should be the prefix: {!string}
lib/commands/xctest.js
Outdated
/** | ||
* List XCTests in a test bundle | ||
* | ||
* @param {string!} bundle Bundle ID |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method should also accept opts
/** | ||
* @typedef {Object} InstallXCTestBundleOpts | ||
* | ||
* @property {xctestApp} xctestBundle Path of the XCTest app (URL or .app) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@property {!string} xctestApp ...
Another push. The IDB failure is unrelated to this PR, I'll need to make a separate PR to fix it. |
lib/commands/xctest.js
Outdated
|
||
/** | ||
* Run an XCTest. Launches a subprocess that runs the XC Test and blocks | ||
* until it is complete. Throws an exception if the subprocess fails. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather move these descriptions into @throws
and @returns
tags
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just some minor comments to the docstrings
Add a feature that allows users to run XCTests (requires IDB installed an
launchWithIdb
capability to be set).Justification
This is useful primarily for cloud users of Appium, as it will allow iOS testers to run their XCTests against a grid of iOS devices (simulators/real). The only requirement for the cloud provider will be that they need to have IDB installed on their machines.