-
-
Notifications
You must be signed in to change notification settings - Fork 425
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
Add endpoints for screen recording support #609
Conversation
lib/commands/recordscreen.js
Outdated
this._recentScreenRecordingPath = null; | ||
} | ||
|
||
const localPath = temp.path({prefix: 'appium', suffix: DEFAULT_EXT}); |
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.
appium-support
supports this: https://github.com/appium/appium-support/blob/master/lib/tempdir.js
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.
shall I use tempdir.path instead? I find it a bit redundant, since I don't need a whole directory to be created, but rather just a single file
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 tall temporary file management be done in a single place, yes.
lib/commands/recordscreen.js
Outdated
log.errorAndThrow(`'${binaryName}' binary is not found in PATH. Make sure it is present on the system. ` + | ||
`Check https://github.com/WPO-Foundation/xrecord for more details.`); | ||
} | ||
args.push('--quicktime', |
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.
Both real device and simulator case have base arguments, so pushing is not necessary, in the same way as binaryName
is handled.
let binaryName;
let args;
if (this.isRealDevice()) {
binaryName = REAL_DEVICE_BINARY;
// ...
args = {
'--quicktime',
`--id="${this.opts.device.udid}"`,
`--out="${localPath}"`,
`--force`,
};
// ...
} else {
binaryName = SIMULATOR_BINARY;
args = {
'simctl',
'io',
this.opts.device.udid,
'recordVideo',
};
// ...
}
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.
for me it is just a matter of style. You know how I like constants :)
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.
But it is logically not a constant. :)
* Initial implementation of Simulator screen recorder * Apply minor fixes * Tune timeouts * Tune regexp * Fix setting options * Add tests and do basic refactoring * Update appium-support version * Address review comments * Tune the logic for Simulator recorder * Update functional test * Set proper defaults and tune timeout calculation * Tune error messages
Added primitives to support screen recording using simctl for Simulators and xrecord tool for real devices.
This PR depends on appium/appium-support#61