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

Fix tests in main #4174

Merged
merged 6 commits into from
Aug 13, 2024
Merged

Fix tests in main #4174

merged 6 commits into from
Aug 13, 2024

Conversation

aboedo
Copy link
Member

@aboedo aboedo commented Aug 12, 2024

Fixes the tests that were failing in main, which you can see in #4172

@aboedo aboedo added the test label Aug 12, 2024
@aboedo aboedo self-assigned this Aug 12, 2024
@aboedo
Copy link
Member Author

aboedo commented Aug 12, 2024

@RCGitBot please test

@aboedo
Copy link
Member Author

aboedo commented Aug 12, 2024

having any references to ContentUnavailableView simply won't compile in Xcode 14, since that entity was created using a newer iOS SDK. I'll have to update the code to account for that

@aboedo
Copy link
Member Author

aboedo commented Aug 12, 2024

@RCGitBot please test

@aboedo
Copy link
Member Author

aboedo commented Aug 12, 2024

@RevenueCat/coresdk I think for the iOS 14 and 15 tests that are currently failing, we need to update snapshots?

@aboedo aboedo requested a review from a team August 12, 2024 20:25
@@ -512,7 +512,7 @@ jobs:
spm-revenuecat-ui-ios-16:
executor:
name: macos-executor
xcode_version: '14.3.0'
xcode_version: '15.4.0'

Copy link
Member Author

Choose a reason for hiding this comment

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

this one didn't need Xcode 14, and we do intend to move to 15, so I figured it was worth moving it over

@@ -653,7 +653,7 @@ jobs:
run-test-ios-16:
executor:
name: macos-executor
xcode_version: '14.3.0'
xcode_version: '15.4.0'
Copy link
Member Author

Choose a reason for hiding this comment

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

same here

Comment on lines +33 to +35
2D2AFE8D2C6A834D00D1B0B4 /* TestData.swift in Sources */ = {isa = PBXBuildFile; fileRef = 887A5FDA2C1D037000E1A461 /* TestData.swift */; };
2D2AFE8F2C6A9D8700D1B0B4 /* CompatibilityContentUnavailableView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D2AFE8E2C6A9D8700D1B0B4 /* CompatibilityContentUnavailableView.swift */; };
2D2AFE912C6A9EF500D1B0B4 /* Binding+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D2AFE902C6A9EF500D1B0B4 /* Binding+Extensions.swift */; };
Copy link
Member Author

Choose a reason for hiding this comment

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

some of these files were added into the SPM package but not the Xcode project, so they wouldn't show up in the navigator and they'd fail to compile

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice find. I wish Xcode had the ability to add all sources in a particular path like SPM allows...

Copy link
Member Author

Choose a reason for hiding this comment

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

+100

Comment on lines -22 to 25
@available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *)
@available(iOS 15.0, *)
@available(macOS, unavailable)
@available(tvOS, unavailable)
@available(watchOS, unavailable)
Copy link
Member Author

Choose a reason for hiding this comment

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

this availability was redundant and we were marking it first available and then unavailable for the same platforms

description: Text(description)
)
if #available(iOS 17.0, *) {
#if swift(>=5.9)
Copy link
Member Author

Choose a reason for hiding this comment

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

this is an old trick that's useful for these Xcode transitions - you point to the min swift version that's packed with the newest xcode, and it effectively works as "if xcode is greater than 15"

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool!

Comment on lines -157 to 164
### ios build_swift_api_tester
### ios run_api_tests

```sh
[bundle exec] fastlane ios build_swift_api_tester
[bundle exec] fastlane ios run_api_tests
```

build Swift API tester

### ios build_objc_api_tester

```sh
[bundle exec] fastlane ios build_objc_api_tester
```

build ObjC API tester

### ios build_custom_entitlement_computation_api_tester

```sh
[bundle exec] fastlane ios build_custom_entitlement_computation_api_tester
```

build CustomEntitlementComputation API tester

### ios build_revenuecatui_api_tester

```sh
[bundle exec] fastlane ios build_revenuecatui_api_tester
```

build RevenueCatUI API tester
run API Tests

Copy link
Member Author

Choose a reason for hiding this comment

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

just automatic and left over from fastlane lane updates from other PRs

Copy link
Contributor

@jamesrb1 jamesrb1 left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +33 to +35
2D2AFE8D2C6A834D00D1B0B4 /* TestData.swift in Sources */ = {isa = PBXBuildFile; fileRef = 887A5FDA2C1D037000E1A461 /* TestData.swift */; };
2D2AFE8F2C6A9D8700D1B0B4 /* CompatibilityContentUnavailableView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D2AFE8E2C6A9D8700D1B0B4 /* CompatibilityContentUnavailableView.swift */; };
2D2AFE912C6A9EF500D1B0B4 /* Binding+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D2AFE902C6A9EF500D1B0B4 /* Binding+Extensions.swift */; };
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice find. I wish Xcode had the ability to add all sources in a particular path like SPM allows...

description: Text(description)
)
if #available(iOS 17.0, *) {
#if swift(>=5.9)
Copy link
Contributor

Choose a reason for hiding this comment

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

Cool!

Copy link
Contributor

@vegaro vegaro left a comment

Choose a reason for hiding this comment

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

it all makes sense to me

@aboedo aboedo marked this pull request as ready for review August 13, 2024 18:58
@aboedo
Copy link
Member Author

aboedo commented Aug 13, 2024

Note: this fixes most tests but it doesn't fix them all, not sure what's happening with iOS 14 and 16 tests. Maybe need to generate snapshots again?

Merging for the time being since it does leave us in a better place than where we started before the PR

@aboedo aboedo merged commit d0dbbdc into main Aug 13, 2024
31 of 33 checks passed
@aboedo aboedo deleted the andy/fix_tests branch August 13, 2024 18:59
joshdholtz added a commit that referenced this pull request Aug 22, 2024
**This is an automatic release.**

### New Features
* Price rounding logic (#4132) via James Borthwick (@jamesrb1)
### Bugfixes
* [Customer Center] Migrate to List style (#4190) via Cody Kerns
(@codykerns)
* [Paywalls] Improve locale consistency (#4158) via Josh Holtz
(@joshdholtz)
* Set Paywalls Tester deployment target to iOS 15 (#4196) via James
Borthwick (@jamesrb1)
* [Customer Center] Hide Contact Support button if URL can't be created
(#4192) via Cesar de la Vega (@vegaro)
* Fix the setting for SKIP_INSTALL in Xcode project (#4195) via Andy
Boedo (@aboedo)
* [Customer Center] Improving customer center buttons (#4165) via Cody
Kerns (@codykerns)
* Revert workaround for iOS 18 beta 5 SwiftUI crash (#4173) via Mark
Villacampa (@MarkVillacampa)
* [Paywalls] Make iOS version calculation lazy (#4163) via Mark
Villacampa (@MarkVillacampa)
* [Paywalls] Observe `PurchaseHandler` when owned externally (#4097) via
James Borthwick (@jamesrb1)
### Other Changes
* [Customer Center] Clean up colors in WrongPlatformView and
NoSubscriptionsView (#4204) via Cesar de la Vega (@vegaro)
* Fix failing `all-tests` and retry more flaky tests (#4188) via Josh
Holtz (@joshdholtz)
* Compatibility content unavailable improvements (#4197) via James
Borthwick (@jamesrb1)
* Create lane to enable customer center (#4191) via Cesar de la Vega
(@vegaro)
* XCFramework artifacts in CircleCI (#4189) via Andy Boedo (@aboedo)
* [Customer Center] CustomerCenterViewModel checks whether the app is
the latest version (#4169) via JayShortway (@JayShortway)
* export RevenueCatUI xcframework (#4172) via Andy Boedo (@aboedo)
* Corrects references from ManageSubscriptionsButtonStyle to
ButtonsStyle. (#4186) via JayShortway (@JayShortway)
* Speed up carthage installation tests (#4184) via Andy Boedo (@aboedo)
* Customer center improvements (#4166) via James Borthwick (@jamesrb1)
* replace `color(from colorInformation:)` global with extension (#4183)
via Andy Boedo (@aboedo)
* Fix tests in main (#4174) via Andy Boedo (@aboedo)
* Enable customer center tests (#4171) via James Borthwick (@jamesrb1)
* [Customer Center] Initial implementation (#3967) via Cesar de la Vega
(@vegaro)

---------

Co-authored-by: Josh Holtz <me@joshholtz.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants