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

Update to Xcode 16 #549

Merged
merged 9 commits into from
Oct 28, 2024
Merged

Update to Xcode 16 #549

merged 9 commits into from
Oct 28, 2024

Conversation

etoledom
Copy link
Contributor

Closes #413

Description

This PR takes the work from #414, trying to fix the issue with circular corner radius on snapshot tests.

The way to fix this I found is to add this line on the Avatar view layer:
avatarImageView.layer.cornerCurve = .circular
And it seems to work.

There are also small differences on how things are rendered, so mostly all snapshot need re-drawing (mostly fonts).
It's important to check that the differences are not noticeable by the eye.

Testing Steps

  • CI: 🍏
  • Run GravatarUI unit tests locally (on iPhone SE)

@@ -65,6 +65,7 @@ class DefaultAvatarProvider: AvatarProviding {
private func applyCornerRadius() {
guard !skipStyling else { return }
avatarImageView.layer.cornerRadius = avatarCornerRadius
avatarImageView.layer.cornerCurve = .circular
Copy link
Contributor

Choose a reason for hiding this comment

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

Snapshots look nice as far as I can tell.

Copy link
Contributor

@pinarol pinarol left a comment

Choose a reason for hiding this comment

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

LGTM 👍
(I ran snapshot tests in my local with iPhone SE 3rd gen. iOS 18.0)

@wpmobilebot
Copy link

wpmobilebot commented Oct 28, 2024

Gravatar SwiftUI Prototype Build📲 You can test the changes from this Pull Request in Gravatar SwiftUI Prototype Build by scanning the QR code below to install the corresponding build.
App NameGravatar SwiftUI Prototype Build Gravatar SwiftUI Prototype Build
Build Number1641
Version1.0
Bundle IDcom.automattic.gravatar-sdk-demo-swiftui.prototype-build
Commit49c2826
App Center BuildGravatar SDK Demo - SwiftUI #335
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link

wpmobilebot commented Oct 28, 2024

Gravatar UIKit Prototype Build📲 You can test the changes from this Pull Request in Gravatar UIKit Prototype Build by scanning the QR code below to install the corresponding build.
App NameGravatar UIKit Prototype Build Gravatar UIKit Prototype Build
Build Number1641
Version1.0
Bundle IDcom.automattic.gravatar-sdk-demo-uikit.prototype-build
Commit49c2826
App Center BuildGravatar SDK Demo - UIKit #336
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

Copy link
Contributor

@andrewdmontgomery andrewdmontgomery left a comment

Choose a reason for hiding this comment

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

Excellent 🚀

@@ -3,6 +3,6 @@
# This file is `source`'d before calling `buildkite-agent pipeline upload`, and can be used
# to set up some variables that will be interpolated in the `.yml` pipeline before uploading it.

export IMAGE_ID=$(echo "xcode-$(cat .xcode-version)")
export IMAGE_ID="xcode-16.0-v7"
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, nice.

Comment on lines +10 to +13
override func invokeTest() {
withSnapshotTesting(record: .failed) {
super.invokeTest()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't know about this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is new after updating the package.

If the snapshot comparison fails, there will be a new recording right away, but the test will also fail.

I think this is good to see right away what's de difference, and we can choose to save it (just run the test again it will succeed), or choose to make changes if the diff is unwanted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andrewdmontgomery andrewdmontgomery mentioned this pull request Oct 28, 2024
@andrewdmontgomery andrewdmontgomery merged commit 3cd03e3 into trunk Oct 28, 2024
11 checks passed
@andrewdmontgomery andrewdmontgomery deleted the etoledom/xcode-16 branch October 28, 2024 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Xcode 16.0
4 participants