swiftcsv / SwiftCSV

CSV parser for Swift
MIT License
947 stars 190 forks source link

Tests fail in Xcode due to ResourceHelper using wrong path #130

Closed Diggory closed 11 months ago

Diggory commented 1 year ago

Hello,

I find that when running the tests in Xcode they crash (they don't crash when running the tests from the shell using swift test):

SwiftCSVTests/PerformanceTest.swift:16: Fatal error: Unexpectedly found nil while unwrapping an Optional value
2023-08-24 07:52:43.102488+0100 xctest[3765:8434382] SwiftCSVTests/PerformanceTest.swift:16: Fatal error: Unexpectedly found nil while unwrapping an Optional value

This appears to be due to ResourceHelper.url() which tries to get test CSV files from the test bundle via a hard-coded path } else if let realBundle = Bundle(path: "\(bundle.bundlePath)/../../../../SwiftCSVTests") { . This fails because Xcode stores derived data in the user's library folder, not the project folder these days.

I have a branch which gets rid of ResourceHelper entirely and instead uses Bundle.module.url(forResource:, withExtension: ) however that requires bumping the swift-tools version in the package manifest to 5.5 (from 5.0 in main).

Would that be acceptable? I assume that this would restrict building the package to using swift version 5.5 or later. (5.5 was released in Sept 2021 and requires Xcode 13 on macOS which requires Big Sur.)

(At the time of writing, the latest version of Swift is 5.8.1)

DivineDominion commented 1 year ago

Yeah, Swift 5.5 is ok. Deployment targets are more important. Thanks for checking this out!

I do use sth. like

#if SWIFT_PACKAGE
let resourceBundle = Bundle.module
#else
let resourceBundle = Bundle.main
#endif

in our apps as well. Should've thought of this :)

DivineDominion commented 12 months ago

@Diggory If you should go ahead with this, that's be a good point to release a new version of the library with your other fixes, IMHO

Diggory commented 12 months ago

Sorry - I was on holiday. I'll make a PR.

D

DivineDominion commented 12 months ago

No need to be sorry for taking a week off! Was just looking at things by chance, didn't meant to apply any pressure