Copilot commented on code in PR #153:
URL: https://github.com/apache/arrow-swift/pull/153#discussion_r3007062746
##########
Package.swift:
##########
@@ -70,6 +71,11 @@ let package = Package(
.testTarget(
name: "ArrowTests",
dependencies: ["Arrow", "ArrowC"],
+ resources: [
+ .copy("testdata_bool.arrow"),
+ .copy("testdata_double.arrow"),
+ .copy("testdata_struct.arrow")
+ ],
Review Comment:
The test target declares `.copy("testdata_*.arrow")` resources, but those
`.arrow` files are not present under `Tests/ArrowTests/` in this branch.
SwiftPM validates resource paths when loading the package, so this will break
`swift build`/dependency resolution unless the files are checked in (or
generated by a build tool plugin). Either commit these files (and keep them
tracked), or switch to a generation approach that guarantees the resources
exist before SwiftPM evaluates the package graph (e.g., a SwiftPM build-tool
plugin), otherwise consumers will not be able to build the package.
```suggestion
```
##########
Tests/ArrowTests/IPCTests.swift:
##########
@@ -106,8 +106,12 @@ func checkStructRecordBatch(_ result:
Result<ArrowReader.ArrowReaderResult, Arro
return recordBatches
}
-func currentDirectory(path: String = #file) -> URL {
- return URL(fileURLWithPath: path).deletingLastPathComponent()
+func testDataURL(_ filename: String) -> URL {
+ guard let url = Bundle.module.url(forResource: filename, withExtension:
"arrow") else {
+ fatalError("Test data file \(filename).arrow not found in bundle. "
+ + "Run the Go data generator first.")
Review Comment:
Using `fatalError` here will crash the entire test process if the resource
is missing, which is harder to diagnose and can mask other failures. Consider
making `testDataURL` `throws` and using `XCTUnwrap`/`XCTFail` so the failure is
reported as a normal XCTest failure instead of aborting the run.
```suggestion
XCTFail("Test data file \(filename).arrow not found in bundle. "
+ "Run the Go data generator first.")
// Return a neutral placeholder URL so the test process does not
crash.
return URL(fileURLWithPath: "/dev/null")
```
--
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]
