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

Swift 6 Compatibility #55

Merged
merged 10 commits into from
Aug 13, 2024
Merged

Swift 6 Compatibility #55

merged 10 commits into from
Aug 13, 2024

Conversation

Supereg
Copy link
Member

@Supereg Supereg commented Aug 11, 2024

Swift 6 Compatibility

♻️ Current situation & Problem

This PR enables strict concurrency and makes SpeziOnboarding compatible with Swift 6.

⚙️ Release Notes

  • Swift 6 Compatibility.
  • Fixed an issue with onboarding identifiers not working with Xcode 16

📚 Documentation

--

✅ Testing

Unit tests were adjusted to be more stable and run slightly faster.

📝 Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

@Supereg Supereg requested a review from PSchmiedmayer August 11, 2024 08:41
@Supereg
Copy link
Member Author

Supereg commented Aug 11, 2024

@PSchmiedmayer we currently skip the export consent form tests after opening the share sheet. With the current beta simulators the save button just stays disabled. Generally, the Files app is completely broken (you cannot create Folders etc).
Therefore, the test is skipped if we encounter a disabled "Save" button. Nonetheless, the test itself was improved to be more stable when running on 17.5 versions.

Looking at the current build output, it seems there are only iOS 18 simulators available, even when building with Xcode 15. Is there anyway to keep the iOS 17.5 simulators even though if we might need to manually tag the version in the destination for SpeziOnboarding. Otherwise, it seems like the PDF export has to remain untested till Apple fixes that issue (if they do 🙂).

Copy link

codecov bot commented Aug 11, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 77.15%. Comparing base (8d6dda3) to head (83e51b7).

Files Patch % Lines
...nboarding/OnboardingIdentifiableViewModifier.swift 62.50% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #55      +/-   ##
==========================================
- Coverage   77.39%   77.15%   -0.24%     
==========================================
  Files          23       23              
  Lines        1185     1190       +5     
==========================================
+ Hits          917      918       +1     
- Misses        268      272       +4     
Files Coverage Δ
...nboarding/ConsentView/ConsentDocument+Export.swift 98.06% <100.00%> (ø)
Sources/SpeziOnboarding/OnboardingDataSource.swift 100.00% <100.00%> (ø)
...ding/OnboardingFlow/OnboardingStepIdentifier.swift 100.00% <100.00%> (ø)
...oarding/OnboardingFlow/OnboardingViewBuilder.swift 53.85% <100.00%> (ø)
...nboarding/OnboardingIdentifiableViewModifier.swift 72.73% <62.50%> (-12.98%) ⬇️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d6dda3...83e51b7. Read the comment docs.

Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Supereg Unfortunately it seems like automatically using an iOS Simulator version higher than the Xcode version was bundled with is a regression in Xcode. Only saw this with iOS 18 simulators so far.

I agree that we should disable the tests for now; they mainly just test the files app anyways (which Apple doesn't seem to do 😄) and it is not our responsibility to add UI tests for their applications.

I did a similar thing in #51 and it seemed to have worked there.

Fine with merging this as is; thanks for all the work @Supereg!

@Supereg
Copy link
Member Author

Supereg commented Aug 13, 2024

@Supereg Unfortunately it seems like automatically using an iOS Simulator version higher than the Xcode version was bundled with is a regression in Xcode. Only saw this with iOS 18 simulators so far.

I agree that we should disable the tests for now; they mainly just test the files app anyways (which Apple doesn't seem to do 😄) and it is not our responsibility to add UI tests for their applications.

I did a similar thing in #51 and it seemed to have worked there.

Fine with merging this as is; thanks for all the work @Supereg!

I wrote the tests in a way, that they automatically run again if they detect that saving files is possible again.

In this case it's not about testing the Files App, but performing some validations for the generated PDF document. This is how we, e.g., detected that the signature field was never shown on visionOS.

But we are reworking the PDF export anyways. So it might be less fragile anyways and less platform-dependent anyway. And maybe there is a way to just unit test the export? This would also be much faster to execute.

@Supereg Supereg merged commit ec570fe into main Aug 13, 2024
20 checks passed
@Supereg Supereg deleted the feature/strict-concurrency branch August 13, 2024 06:47
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