Copilot commented on code in PR #153:
URL: https://github.com/apache/arrow-swift/pull/153#discussion_r3007768662
##########
.github/workflows/test.yaml:
##########
@@ -113,3 +113,70 @@ jobs:
github.ref_name == 'main'
run: |
docker compose push ubuntu
+
+ ios:
+ name: iOS
+ runs-on: macos-15
+ timeout-minutes: 15
+ steps:
+ - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd #
v6.0.2
+ - uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 #
v5.5.0
+ with:
+ go-version: "stable"
+ - name: Generate test data
+ run: |
+ cd data-generator/swift-datagen
+ go run .
+ cp *.arrow ../../Tests/ArrowTests/
+ - name: Select iOS Simulator
+ run: |
+ # Pick the first iPhone on the newest available iOS.
+ #
+ # Example pipeline on macos-15 runner
+ #
+ # xcrun simctl list devices available (filtered to iPhone):
+ # -- iOS 18.5 --
+ # iPhone 16 Pro (E8B81AE5-...) (Shutdown)
+ # iPhone 16 (394B9336-...) (Shutdown)
+ # -- iOS 18.6 --
+ # iPhone 16 Pro (37C8BF13-...) (Shutdown)
+ # iPhone 16 (E040B4E8-...) (Shutdown)
+ # -- iOS 26.2 --
+ # iPhone 17 Pro (20A8DB5D-...) (Shutdown)
+ # iPhone 16 (02BBE218-...) (Shutdown)
+ #
+ # -> awk: prefix each iPhone line with its iOS version
+ # 18.5 iPhone 16 Pro (E8B81AE5-...) (Shutdown)
+ # 18.5 iPhone 16 (394B9336-...) (Shutdown)
+ # 18.6 iPhone 16 Pro (37C8BF13-...) (Shutdown)
+ # 26.2 iPhone 17 Pro (20A8DB5D-...) (Shutdown)
+ # 26.2 iPhone 16 (02BBE218-...) (Shutdown)
+ #
+ # -> sort -t. -k1,1nr -k2,2nr: sort by iOS version descending
+ # 26.2 iPhone 17 Pro (20A8DB5D-...) (Shutdown)
+ # 26.2 iPhone 16 (02BBE218-...) (Shutdown)
+ # 18.6 iPhone 16 Pro (37C8BF13-...) (Shutdown)
+ # 18.5 iPhone 16 Pro (E8B81AE5-...) (Shutdown)
+ #
+ # -> head -1: take the first line (newest iOS)
+ # 26.2 iPhone 17 Pro (20A8DB5D-...) (Shutdown)
+ #
+ # -> grep -oE: extract the simulator UDID
+ # 20A8DB5D-3B2A-48CF-BA79-07E054563A31
+ SIMULATOR_ID=$(xcrun simctl list devices available 2>/dev/null | \
+ awk '/-- iOS/{os=$3} /iPhone/{print os, $0}' | \
+ sort -t. -k1,1nr -k2,2nr | head -1 | \
+ grep -oE '[0-9A-Fa-f]{8}-([0-9A-Fa-f]{4}-){3}[0-9A-Fa-f]{12}')
+ if [ -z "$SIMULATOR_ID" ]; then
+ echo "Error: no iPhone simulator found"
+ xcrun simctl list devices available
+ exit 1
+ fi
+ echo "SIMULATOR_ID=$SIMULATOR_ID" >> "$GITHUB_ENV"
+ echo "Selected simulator: $SIMULATOR_ID"
+ - name: Test on iOS Simulator
+ run: |
+ xcodebuild test \
+ -scheme Arrow-Package \
+ -destination "platform=iOS Simulator,id=$SIMULATOR_ID" \
+ -only-testing ArrowTests
Review Comment:
The new iOS CI job only runs `xcodebuild test` with `-only-testing
ArrowTests`, which won’t compile (or run) `ArrowFlightTests` and may not even
build the `ArrowFlight` target. This leaves a gap where the package can be
declared iOS-supported but still regress for ArrowFlight on iOS without CI
catching it. Consider adding an additional `xcodebuild build` step for the
ArrowFlight scheme (or otherwise ensuring all package products compile) on the
selected simulator destination.
```suggestion
-only-testing ArrowTests
- name: Build ArrowFlight for iOS Simulator
run: |
xcodebuild build \
-scheme ArrowFlight \
-destination "platform=iOS Simulator,id=$SIMULATOR_ID"
```
##########
Package.swift:
##########
@@ -23,7 +23,8 @@ import PackageDescription
let package = Package(
name: "Arrow",
platforms: [
- .macOS(.v10_15)
+ .macOS(.v10_15),
+ .iOS(.v15)
],
Review Comment:
Declaring `.iOS(.v15)` at the package level makes it easy for consumers to
expect both `Arrow` and `ArrowFlight` to be usable on iOS. Right now the iOS CI
job only exercises `ArrowTests`, so it’s possible for `ArrowFlight` to drift
out of iOS compatibility unnoticed. Consider either (a) expanding iOS CI to
compile/build the `ArrowFlight` product too, or (b) explicitly documenting that
iOS support applies only to the core `Arrow` library if that’s the intent.
##########
Tests/ArrowTests/IPCTests.swift:
##########
@@ -106,8 +106,14 @@ func checkStructRecordBatch(_ result:
Result<ArrowReader.ArrowReaderResult, Arro
return recordBatches
}
-func currentDirectory(path: String = #file) -> URL {
- return URL(fileURLWithPath: path).deletingLastPathComponent()
+func testDataURL(_ filename: String) throws -> URL {
+ guard let url = Bundle.module.url(forResource: filename, withExtension:
"arrow") else {
+ throw ArrowError.ioError(
+ "Test data file \(filename).arrow not found in bundle. "
+ + "Run the Go data generator first."
Review Comment:
The failure message in `testDataURL` tells contributors to “Run the Go data
generator first,” but the `.arrow` files are now checked into
`Tests/ArrowTests` and bundled as SwiftPM resources. This can be misleading
when the real issue is a missing/incorrect test resource configuration.
Consider updating the message to mention verifying the test resources in
`Package.swift` (and running the generator only when regenerating/updating the
committed testdata).
```suggestion
"Test data file \(filename).arrow not found in test bundle. "
+ "Verify that the .arrow test resources are correctly
declared in Package.swift, "
+ "and run the Go data generator only when regenerating or
updating the committed test data."
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
