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

fix: Add arm64 arch to XctestrunFile path search for simulator #838

Conversation

suzukieita
Copy link

#837

Fixed to check both x86_64 and arm 64 if the device is a simulator.
I added an argument to getXctestrunFileName function but is has a default value, so it doesn't change the original behavior without it.

Copy link

linux-foundation-easycla bot commented Jan 19, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: suzukieita / name: Suzuki Eita (006b348)

return isTvOS(deviceInfo.platformName)
? `WebDriverAgentRunner_tvOS_appletv${deviceInfo.isRealDevice ? `os${version}-arm64` : `simulator${version}-x86_64`}.xctestrun`
Copy link
Member

@KazuCocoa KazuCocoa Jan 19, 2024

Choose a reason for hiding this comment

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

Perhaps you could use:

`simulator${version}-${os.arch() === "arm64" ? "arm64" : "x86_64"}`

instead of check both cases.
The simulator depends on the host, thus this line can guess the proper naming with the host's architecture.

Choose a reason for hiding this comment

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

I agree. The WDA architecture should be selected based on the current host architecture

Copy link
Author

Choose a reason for hiding this comment

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

I see we can check the arch by os.arch(), thank you for letting me know about it. I'll fix it.

Copy link
Author

Choose a reason for hiding this comment

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

By the way, where can I get the os variable? And if it contains the string directly, we may use it like

`simulator${version}-${os.arch()"}`

I checked the DeviceInfo but it doesn't contain the property.

And I also noticed I should change the comment here based on the fix.
https://github.com/appium/WebDriverAgent/blob/master/lib/utils.js#L160-L184

Copy link
Member

Choose a reason for hiding this comment

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

@KazuCocoa KazuCocoa changed the title Add arm64 arch to XctestrunFile path search fix: Add arm64 arch to XctestrunFile path search Jan 19, 2024
@KazuCocoa KazuCocoa changed the title fix: Add arm64 arch to XctestrunFile path search fix: Add arm64 arch to XctestrunFile path search for simulator Jan 19, 2024
@KazuCocoa
Copy link
Member

appium-webdriveragent v5.15.8 has the fix #840
Thank you for the pr

@KazuCocoa KazuCocoa closed this Jan 24, 2024
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.

3 participants