Skip to content

Add endpoints for screen recording support#609

Merged
imurchie merged 12 commits intoappium:masterfrom
mykola-mokhnach:screen_record
Jan 19, 2018
Merged

Add endpoints for screen recording support#609
imurchie merged 12 commits intoappium:masterfrom
mykola-mokhnach:screen_record

Conversation

@mykola-mokhnach
Copy link
Copy Markdown
Contributor

Added primitives to support screen recording using simctl for Simulators and xrecord tool for real devices.

This PR depends on appium/appium-support#61

Comment thread lib/commands/recordscreen.js Outdated
this._recentScreenRecordingPath = null;
}

const localPath = temp.path({prefix: 'appium', suffix: DEFAULT_EXT});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor

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.

Comment thread 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',
Copy link
Copy Markdown
Contributor

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',
  };
  // ...
}

Copy link
Copy Markdown
Contributor Author

@mykola-mokhnach mykola-mokhnach Jan 18, 2018

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 :)

Copy link
Copy Markdown
Contributor

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. :)

@imurchie imurchie merged commit 37ce62d into appium:master Jan 19, 2018
@mykola-mokhnach mykola-mokhnach deleted the screen_record branch January 30, 2018 13:52
khanayan123 pushed a commit to khanayan123/appium-xcuitest-driver that referenced this pull request May 10, 2021
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants