Skip to content
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 enforceFreshSimulatorCreation capability #859

Merged
merged 3 commits into from
Jan 19, 2019
Merged

Conversation

imurchie
Copy link
Contributor

If the system on which Appium is being run has no simulator with the required platform version and name, Appium will create one, and then delete it at the end of the session. If there is a simulator, though, it will just use it, which can cause problems (see appium/appium#12025).

This PR introduces a capability, useNewSimulator, which will force the use of a new sim if no UDID is specified in the caps.

Copy link
Member

@jlipps jlipps left a comment

Choose a reason for hiding this comment

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

to me 'use' is a bit ambiguous, and could potentially mean "if there's a new simulator that already exists, then use it". where what this is really doing is always creating a new sim. maybe something like createSimForSession, createNewSim, makeFreshSim, or similar?

@mykola-mokhnach
Copy link
Contributor

to me 'use' is a bit ambiguous, and could potentially mean "if there's a new simulator that already exists, then use it". where what this is really doing is always creating a new sim. maybe something like createSimForSession, createNewSim, makeFreshSim, or similar?

how about enforceFreshSimulatorCreation ?

lib/driver.js Outdated Show resolved Hide resolved
lib/driver.js Outdated Show resolved Hide resolved
if (!this.opts.platformVersion && this.iosSdkVersion) {
log.info(`No platformVersion specified. Using latest version Xcode supports: '${this.iosSdkVersion}' ` +
`This may cause problems if a simulator does not exist for this platform version.`);
this.opts.platformVersion = this.iosSdkVersion;
}

if (this.opts.noReset) {
Copy link
Contributor

Choose a reason for hiding this comment

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

don't we need to consider noReset?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I honestly could not figure out how this block ever got run. The original code was:

let device = await getExistingSim(this.opts);

// check for an existing simulator
if (device) {
  return {device, realDevice: false, udid: device.udid};
}

// no device of this type exists, so create one
log.info('Simulator udid not provided, using desired caps to create a new simulator');
if (!this.opts.platformVersion && this.iosSdkVersion) {
  log.info(`No platformVersion specified. Using latest version Xcode supports: '${this.iosSdkVersion}' ` +
           `This may cause problems if a simulator does not exist for this platform version.`);
  this.opts.platformVersion = this.iosSdkVersion;
}

if (this.opts.noReset) {
  // Check for existing simulator just with correct capabilities
  let device = await getExistingSim(this.opts);
  if (device) {
    return {device, realDevice: false, udid: device.udid};
  }
}

Which means the noReset section is only different in that the platformVersion, if empty, has been set to the Xcode default.

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume the idea behind this is that we never wanted to create a new simulator if noReset it set. Although, I don't mind the current approach

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll put it back in, since it actually seems to have a use (#520). Needs to be documented in the code, though, since it is unclear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Though I actually think moving the platformVersion check up a little is a better solution.

@@ -17,7 +19,7 @@ const INSTALL_DAEMON_CACHE = 'com.apple.mobile.installd.staging';
* @returns {object} Simulator object associated with the udid passed in.
*/
async function createSim (caps) {
const appiumTestDeviceName = `appiumTest-${caps.deviceName}`;
const appiumTestDeviceName = `appiumTest-${UUID.create().toString().toUpperCase()}-${caps.deviceName}`;
Copy link
Member

Choose a reason for hiding this comment

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

👍

@imurchie imurchie changed the title Add useNewSimulator capability Add enforceFreshSimulatorCreation capability Jan 18, 2019
@imurchie imurchie merged commit fa7084c into master Jan 19, 2019
@imurchie imurchie deleted the isaac-new-sim branch January 19, 2019 11:18
@@ -734,6 +734,12 @@ class XCUITestDriver extends BaseDriver {
return {device, realDevice: true, udid: this.opts.udid};
}

if (!this.opts.platformVersion && this.iosSdkVersion) {
log.info(`No platformVersion specified. Using latest version Xcode supports: '${this.iosSdkVersion}' ` +
Copy link
Contributor

Choose a reason for hiding this comment

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

the latest?

khanayan123 pushed a commit to khanayan123/appium-xcuitest-driver that referenced this pull request May 10, 2021
* Add useNewSimulator capability

* Change cap name to 'enforceFreshSimulatorCreation'

* Move platformVersion check in determinDevice
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.

None yet

4 participants