Skip to content

[image_picker] Fix XCTests in Xcode 15 #5074

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.

Sign up for GitHub

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

Jump to bottom
Merged
auto-submit merged 2 commits into from
Oct 5, 2023
Merged

[image_picker] Fix XCTests in Xcode 15 #5074

auto-submit merged 2 commits into from
Oct 5, 2023

Conversation

@stuartmorgan-g
Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g commented Oct 4, 2023

When compiled with Xcode 15, images that can't be loaded by UIImage will cause the codepaths in some XCTest tests to throw exceptions. To address this:

  • Replace the test ICO file with one that UIImage is able to open.
  • Remove the SVG test, since UIImage cannot directly load SVGs.

Fixes flutter/flutter#134973

@stuartmorgan-g
[image_picker] Fix XCTests in Xcode 15
1192d17 
When compiled with Xcode 15, images that can't be loaded by UIImage will
cause the codepaths in some XCTest tests to throw exceptions. To address
this:
- Replace the test ICO file with one that UIImage is able to open.
- Remove the SVG test, since UIImage cannot directly load SVGs.

Fixes flutter/flutter#134973
vashworth
vashworth approved these changes Oct 5, 2023
View reviewed changes
Copy link
Contributor

@vashworth vashworth left a comment

Choose a reason for hiding this comment

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

So SVGs don't work with image_picker at all? If SVGs don't work at all, we should probably file an issue to investigate supporting them / making sure we have proper error messaging.

Also, just curious, do you know what was wrong with the ICO image?

Other than that, LGTM. Thanks for fixing this!

@stuartmorgan-g
Copy link
Collaborator Author

stuartmorgan-g commented Oct 5, 2023 •
edited
Loading

So SVGs don't work with image_picker at all? If SVGs don't work at all, we should probably file an issue to investigate supporting them / making sure we have proper error messaging.

I don't know if they work or not, but this test isn't actually testing them because the item provider returns nil since UIImage doesn't support SVGs directly. From some looking around, SVG support on iOS seems pretty limited, so I'm not sure how easy it would be to validate them. I figured it's obscure enough for the image_picker use case that we could leave any further investigation of SVG support to be custom-issue-report driven.

Also, just curious, do you know what was wrong with the ICO image?

Nope, I looked around a bit and couldn't figure out what the requirements are for macOS/iOS to handle an ICO. I don't think the ICO itself was invalid, I think Apple's libraries (it's not just UIImage; Finder wouldn't show a preview for it and Preview wouldn't load it) just can't handle some subset of the format.

@stuartmorgan-g stuartmorgan-g added override: no versioning needed Override the check requiring version bumps for most changes override: no changelog needed Override the check requiring CHANGELOG updates for most changes labels Oct 5, 2023
@stuartmorgan-g
Copy link
Collaborator Author

stuartmorgan-g commented Oct 5, 2023

Changelog/version override: these changes are test-only, just in a way the tooling doesn't detect.

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 5, 2023
@auto-submit auto-submit bot merged commit e578a16 into flutter:main Oct 5, 2023
@guptavirendra

This comment was marked as off-topic.

Sign in to view
@stuartmorgan-g stuartmorgan-g deleted the image-picker-xcode-15-test-fixes branch October 5, 2023 19:25
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Oct 6, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 6, 2023
@engine-flutter-autoroll
e578a16 [image_picker] Fix XCTests in Xcode 15 (flutter/packages#5074)
ccd8aed
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Oct 6, 2023
@engine-flutter-autoroll
Roll Packages from 6714d50 to e578a16 (2 revisions) (#136074)
a3fb4a1 
flutter/packages@6714d50...e578a16

2023-10-05 [email protected] [image_picker] Fix XCTests in Xcode 15 (flutter/packages#5074)
2023-10-05 [email protected] Roll Flutter from 5122991 to fe0275f (35 revisions) (flutter/packages#5076)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Mairramer pushed a commit to Mairramer/flutter that referenced this pull request Oct 10, 2023
@engine-flutter-autoroll @Mairramer
Roll Packages from 6714d50 to e578a16 (2 revisions) (flutter#136074)
7ed920f 
flutter/packages@6714d50...e578a16

2023-10-05 [email protected] [image_picker] Fix XCTests in Xcode 15 (flutter/packages#5074)
2023-10-05 [email protected] Roll Flutter from 5122991 to fe0275f (35 revisions) (flutter/packages#5076)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vashworth vashworth vashworth approved these changes

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App override: no changelog needed Override the check requiring CHANGELOG updates for most changes override: no versioning needed Override the check requiring version bumps for most changes p: image_picker platform-ios

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

XCTests and XCUITests failing on image_picker package iOS 17 XCode RC1

3 participants

@stuartmorgan-g @guptavirendra @vashworth