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

[image_picker] Fix XCTests in Xcode 15 #5074

Merged

Conversation

stuartmorgan
Copy link
Contributor

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

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
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
Copy link
Contributor Author

stuartmorgan commented Oct 5, 2023

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 stuartmorgan 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
Copy link
Contributor Author

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

@stuartmorgan stuartmorgan 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.

@stuartmorgan stuartmorgan deleted the image-picker-xcode-15-test-fixes branch October 5, 2023 19:25
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 6, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Oct 6, 2023
flutter/packages@6714d50...e578a16

2023-10-05 stuartmorgan@google.com [image_picker] Fix XCTests in Xcode 15 (flutter/packages#5074)
2023-10-05 engine-flutter-autoroll@skia.org 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 flutter-ecosystem@google.com,rmistry@google.com 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
flutter/packages@6714d50...e578a16

2023-10-05 stuartmorgan@google.com [image_picker] Fix XCTests in Xcode 15 (flutter/packages#5074)
2023-10-05 engine-flutter-autoroll@skia.org 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 flutter-ecosystem@google.com,rmistry@google.com 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
HugoOlthof pushed a commit to moneybird/packages that referenced this pull request Dec 13, 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

XCTests and XCUITests failing on image_picker package iOS 17 XCode RC1
3 participants