-
Notifications
You must be signed in to change notification settings - Fork 197
Working on Xcode 13 fixes and SPM tests #412
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
Conversation
Current coverage for BonMot.framework is
|
| Files changed | - | - |
|---|---|---|
| Platform.swift | 25.00% |
🚫 |
| AdaptableTextContainer.swift | 42.00% |
🚫 |
| AdaptiveStyle.swift | 73.99% |
|
| StyleableUIElement.swift | 74.62% |
Powered by xcov
Generated by 🚫 Danger
|
Let's wait for the next beta and see if they fix that known issue. In terms of workarounds, maybe there's something we can do with I think the test is crashing because the test resources aren't included. I think there's an issue or PR open about it already, but it hasn't been touched since package resources became a thing. Worth a revisit. The failing tests you mention are currently disabled on the platforms you mention, I believe. I couldn't find a good way to make them pass everywhere. I don't know what control we have over that in SPM tests. Maybe we can use I think SPM supports executable targets with The breaking version changes sound acceptable. Loving it! |
|
I see you're already doing the resources stuff. Not sure what's wrong, then. Check the resulting bundle. I think the parent folder is copied in intact, and that needs to be included in the search for a particular file. Also, I deprecated something recently and renamed it because of a typo. Since this is a breaking change, let's go ahead and delete the deprecated one just to clean things up. And finally, I'd prefer not to bump the version number in this PR. Version bumps should be separate IMO. But it's not a strong preference, so stick with this if you prefer to avoid the extra paperwork, as it were. |
|
The reason I added the version bump in this PR was because the podspec itself required a minimum iOS version change as well. I agree that typically that part should be handled separately. XCTSkip might be the ticket, because I don't think SPM has a way to skip tests without it. As far as the SPM test resource bundle issue, it should be working, and does work when using Xcode's SPM integration, just not the command line SPM via |
|
@ZevEisenberg That resource issue is that xcasset catalogs are not compiled into Had to add some more |
@ZevEisenberg Building on #411 I had to revert your changes to
featureIdentifierandtypeIdentifierto get things to compile on Xcode 12 again. It crashes at runtime in Xcode 13b2 but it appears that it's a known bug (79090498) that will probably be fixed soon in a later release. Not sure if there's another workaround.While cleaning things up I also added the ability to run tests via Swift Package Manager (superceding #382), however there are still issues there:
swift testit will crash because it can't find a resourcePackage.swiftin Xcode and running SPM tests that way, it will run the tests, but a few of them are failing (FontInspectorTestsfor iOS, and alsoImageTintingTestsfor macOS)This PR also contains breaking changes to the minimum requirements: