-
-
Notifications
You must be signed in to change notification settings - Fork 426
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
fix xctestrun file detection when useXctestrunFile is true #903
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
f8759c6
fix xctestrun path
KazuCocoa 77f03e3
tweak handking sdk version
KazuCocoa 36c56e7
tuen a bit
KazuCocoa 1f2bcc5
update caps in readme
KazuCocoa 8fce38c
add a test to raise an error
KazuCocoa 86dbac8
move control flow into string
KazuCocoa 1725e47
fix reviews
KazuCocoa 6e6eefd
add a tips for building module
KazuCocoa File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -266,6 +266,7 @@ class XCUITestDriver extends BaseDriver { | |
this.opts.device = device; | ||
this.opts.udid = udid; | ||
this.opts.realDevice = realDevice; | ||
this.opts.iosSdkVersion = null; // For WDA and xcodebuild | ||
|
||
if (_.isEmpty(this.xcodeVersion) && (!this.opts.webDriverAgentUrl || !this.opts.realDevice)) { | ||
// no `webDriverAgentUrl`, or on a simulator, so we need an Xcode version | ||
|
@@ -274,7 +275,8 @@ class XCUITestDriver extends BaseDriver { | |
log.info(`Xcode version set to '${this.xcodeVersion.versionString}' ${tools}`); | ||
|
||
this.iosSdkVersion = await getAndCheckIosSdkVersion(); | ||
log.info(`iOS SDK Version set to '${this.iosSdkVersion}'`); | ||
this.opts.iosSdkVersion = this.iosSdkVersion; // Pass to xcodebuild | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
log.info(`iOS SDK Version set to '${this.opts.iosSdkVersion}'`); | ||
} | ||
this.logEvent('xcodeDetailsRetrieved'); | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
import { getXctestrunFilePath } from '../../../lib/wda/utils'; | ||
import chai from 'chai'; | ||
import chaiAsPromised from 'chai-as-promised'; | ||
import { withMocks } from 'appium-test-support'; | ||
import { fs } from 'appium-support'; | ||
import path from 'path'; | ||
import { fail } from 'assert'; | ||
|
||
chai.should(); | ||
chai.use(chaiAsPromised); | ||
|
||
describe('utils', function () { | ||
describe('#getXctestrunFilePath', withMocks({fs}, function (mocks) { | ||
const platformVersion = '12.0'; | ||
const sdkVersion = '12.2'; | ||
const udid = 'xxxxxyyyyyyzzzzzz'; | ||
const bootstrapPath = 'path/to/data'; | ||
|
||
afterEach(function () { | ||
mocks.verify(); | ||
}); | ||
|
||
it('should return sdk based path with udid', async function () { | ||
mocks.fs.expects('exists') | ||
.withExactArgs(path.resolve(`${bootstrapPath}/${udid}_${sdkVersion}.xctestrun`)) | ||
.returns(true); | ||
mocks.fs.expects('copyFile') | ||
.never(); | ||
const deviceInfo = {isRealDevice: true, udid, platformVersion}; | ||
await getXctestrunFilePath(deviceInfo, sdkVersion, bootstrapPath) | ||
.should.eventually.equal(path.resolve(`${bootstrapPath}/${udid}_${sdkVersion}.xctestrun`)); | ||
}); | ||
|
||
it('should return sdk based path without udid, copy them', async function () { | ||
mocks.fs.expects('exists') | ||
.withExactArgs(path.resolve(`${bootstrapPath}/${udid}_${sdkVersion}.xctestrun`)) | ||
.returns(false); | ||
mocks.fs.expects('exists') | ||
.withExactArgs(path.resolve(`${bootstrapPath}/WebDriverAgentRunner_iphoneos${sdkVersion}-arm64.xctestrun`)) | ||
.returns(true); | ||
mocks.fs.expects('copyFile') | ||
.withExactArgs( | ||
path.resolve(`${bootstrapPath}/WebDriverAgentRunner_iphoneos${sdkVersion}-arm64.xctestrun`), | ||
path.resolve(`${bootstrapPath}/${udid}_${sdkVersion}.xctestrun`) | ||
) | ||
.returns(true); | ||
const deviceInfo = {isRealDevice: true, udid, platformVersion}; | ||
await getXctestrunFilePath(deviceInfo, sdkVersion, bootstrapPath) | ||
.should.eventually.equal(path.resolve(`${bootstrapPath}/${udid}_${sdkVersion}.xctestrun`)); | ||
}); | ||
|
||
it('should return platform based path', async function () { | ||
mocks.fs.expects('exists') | ||
.withExactArgs(path.resolve(`${bootstrapPath}/${udid}_${sdkVersion}.xctestrun`)) | ||
.returns(false); | ||
mocks.fs.expects('exists') | ||
.withExactArgs(path.resolve(`${bootstrapPath}/WebDriverAgentRunner_iphonesimulator${sdkVersion}-x86_64.xctestrun`)) | ||
.returns(false); | ||
mocks.fs.expects('exists') | ||
.withExactArgs(path.resolve(`${bootstrapPath}/${udid}_${platformVersion}.xctestrun`)) | ||
.returns(true); | ||
mocks.fs.expects('copyFile') | ||
.never(); | ||
const deviceInfo = {isRealDevice: false, udid, platformVersion}; | ||
await getXctestrunFilePath(deviceInfo, sdkVersion, bootstrapPath) | ||
.should.eventually.equal(path.resolve(`${bootstrapPath}/${udid}_${platformVersion}.xctestrun`)); | ||
}); | ||
|
||
it('should return platform based path without udid, copy them', async function () { | ||
mocks.fs.expects('exists') | ||
.withExactArgs(path.resolve(`${bootstrapPath}/${udid}_${sdkVersion}.xctestrun`)) | ||
.returns(false); | ||
mocks.fs.expects('exists') | ||
.withExactArgs(path.resolve(`${bootstrapPath}/WebDriverAgentRunner_iphonesimulator${sdkVersion}-x86_64.xctestrun`)) | ||
.returns(false); | ||
mocks.fs.expects('exists') | ||
.withExactArgs(path.resolve(`${bootstrapPath}/${udid}_${platformVersion}.xctestrun`)) | ||
.returns(false); | ||
mocks.fs.expects('exists') | ||
.withExactArgs(path.resolve(`${bootstrapPath}/WebDriverAgentRunner_iphonesimulator${platformVersion}-x86_64.xctestrun`)) | ||
.returns(true); | ||
mocks.fs.expects('copyFile') | ||
.withExactArgs( | ||
path.resolve(`${bootstrapPath}/WebDriverAgentRunner_iphonesimulator${platformVersion}-x86_64.xctestrun`), | ||
path.resolve(`${bootstrapPath}/${udid}_${platformVersion}.xctestrun`) | ||
) | ||
.returns(true); | ||
|
||
const deviceInfo = {isRealDevice: false, udid, platformVersion}; | ||
await getXctestrunFilePath(deviceInfo, sdkVersion, bootstrapPath) | ||
.should.eventually.equal(path.resolve(`${bootstrapPath}/${udid}_${platformVersion}.xctestrun`)); | ||
}); | ||
|
||
it('should raise an exception because of no files', async function () { | ||
const expected = path.resolve(`${bootstrapPath}/WebDriverAgentRunner_iphonesimulator${sdkVersion}-x86_64.xctestrun`); | ||
mocks.fs.expects('exists').exactly(4).returns(false); | ||
|
||
const deviceInfo = {isRealDevice: false, udid, platformVersion}; | ||
try { | ||
await getXctestrunFilePath(deviceInfo, sdkVersion, bootstrapPath); | ||
fail(); | ||
} catch (err) { | ||
err.message.should.equal(`If you are using 'useXctestrunFile' capability then you need to have a xctestrun file (expected: '${expected}')`); | ||
} | ||
}); | ||
})); | ||
}); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why not just use
this.opts.iosSdkVersion
and do away withthis.iosSdkVersion
.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.
According to
appium-xcuitest-driver/lib/driver.js
Lines 165 to 176 in 6e6eefd
this.iosSdkVersion
is imported from ios-driver. thus, I addedthis.opts.iosSdkVersion
keepingthis.iosSdkVersion
to avoid affection to this.iosSdkVersion.I replaced this.iosSdkVersion to this.opts.iosSdkVersion once and ran some tests without issues, but I thought the change should have created as another PR.
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'm fine with that.