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

Android support #437

Merged
merged 10 commits into from
Feb 19, 2025
Merged

Android support #437

merged 10 commits into from
Feb 19, 2025

Conversation

marcprux
Copy link
Contributor

@marcprux marcprux commented Jan 6, 2025

This PR adds CI support for building and testing on Android using the swift-android-action. You can see a successful CI run at https://github.com/marcprux/Yams/actions

Everything build out of the box, but the measure function in PerformanceTests.swift needed to be disabled due to high emulator variance in tests (and the inability to configure the the maximum tolerated standard deviation on non-Darwin platforms; see swiftlang/swift-corelibs-xctest#506).

@lynchsft
Copy link
Collaborator

Hi Marc,

I'd love to add support for Swift on Android.

One ask please. Let's provide a more forward-looking bridge to better self.measure functionality on non-Darwin platforms.
Would you please:

  1. implement a basic wall-clock duration calculation in the Android measure function and simply print the duration (formatted) to std out?
  2. Add a url to your PR in corelibs-xctest to the comment in the Android measure function.

Thank you!

@marcprux
Copy link
Contributor Author

OK, I've implemented this and made the comment. My output of the duration is just the TimeInterval, because that is how the standard XCTest measure function outputs it. E.g.:

Test Case '-[YamsTests.PerformanceTests testUsingSwiftDecodableWithUTF8]' measured [CPU Cycles, kC] average: 16242.921 …

@lynchsft
Copy link
Collaborator

The lint failed on your variable name t.
Also, thank you for the comment in the corelibs-xctest project. But actually I was intending to link the other direction: In the comment that starts // the hardwired max standard would you please add a URL to your proposed change in xctest. We'll want to track that effort over time to remove this workaround when possible.

Thank you!

@marcprux
Copy link
Contributor Author

Ahh, understood. Both should be addressed now.

@lynchsft
Copy link
Collaborator

Alll checks pass! :)
This is a nominal approve ✅. Please update the changelog per the CONTRIBUTING guide. 🙏🏻

@marcprux
Copy link
Contributor Author

Please update the changelog per the CONTRIBUTING guide.

Apologies for missing that. Done in 8366191.

@lynchsft lynchsft merged commit 1a7a58b into jpsim:main Feb 19, 2025
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