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.

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

feat: adding vision os preview #3898

Merged
merged 14 commits into from
Oct 21, 2024
Merged

feat: adding vision os preview #3898

merged 14 commits into from
Oct 21, 2024

Conversation

harsh62
Copy link
Member

@harsh62 harsh62 commented Oct 17, 2024 •
edited
Loading

Issue #

Rebased visionos-preview branch with main.Based on customer feedback, we have decided to push visionos-preview on main.

Please note that VisionOS support is still in preview, we are just making this merge to make it more accessible to customers by pushing it into our release pipelines. This way customers using Amplify on VisionOS would get other updates that are made on main.

Description

Integration Tests (Except DataStore & API)
Integration Tests | API - All
Integration Tests | DataStore - All

General Checklist

  • Added new tests to cover change, if needed
  • Build succeeds with all target using Swift Package Manager
  • All unit tests pass
  • All integration tests pass
  • Security oriented best practices and standards are followed (e.g. using input sanitization, principle of least privilege, etc)
  • Documentation update for the change if required
  • PR title conforms to conventional commit style
  • New or updated tests include Given When Then inline code documentation and are named accordingly testThing_condition_expectation()
  • If breaking change, documentation/changelog update with migration instructions

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@harsh62 harsh62 marked this pull request as ready for review October 21, 2024 02:00
@harsh62 harsh62 requested review from a team as code owners October 21, 2024 02:00
ruisebas
ruisebas reviewed Oct 21, 2024
View reviewed changes
.../AWSPinpointAnalyticsPluginIntegrationTests/AWSPinpointAnalyticsPluginIntegrationTests.swift
@@ -91,21 +94,6 @@ class AWSPinpointAnalyticsPluginIntergrationTests: XCTestCase {
try await endpointClient.updateEndpointProfile(with: currentProfile)
}

/// Run this test when the number of endpoints for the userId exceeds the limit.
Copy link
Member

Choose a reason for hiding this comment

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

Let's add this back to save our future selves some head scratching :P

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 for some reason actually failing the build for the target on the runner. Thats why I removed it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added it back commented with a TODO.

Copy link
Member

Choose a reason for hiding this comment

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

That is indeed very weird, since this is not really a test but just a method 🤷‍♂️

Amplify/Core/Configuration/Internal/Amplify+Reset.swift Outdated Show resolved Hide resolved
AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignOut/ShowHostedUISignOut.swift Outdated Show resolved Hide resolved
AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignOut/ShowHostedUISignOut.swift Outdated Show resolved Hide resolved
...fyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/CodeGen/Data/SignOutEventData.swift Outdated Show resolved Hide resolved
...sts/AWSDataStorePluginTests/Sync/MutationQueue/MutationIngesterConflictResolutionTests.swift Outdated Show resolved Hide resolved
...sts/CoreMLPredictionsPluginUnitTests/DependencyTests/CoreMLNaturalLanguageAdapterTests.swift
@@ -53,6 +53,11 @@ class CoreMLNaturalLanguageAdapterTests: XCTestCase {
/// - I should get back correct tokens
///
func testSyntaxToken() {
#if !os(visionOS)
// TODO: Test failing on visionOS.
Copy link
Member

Choose a reason for hiding this comment

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

IIRC the CoreML tests are failing in all platforms, that's why they are not being run by the unit tests workflow. See #3655

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we fix the tests or keep the TODO for future?

Copy link
Member

Choose a reason for hiding this comment

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

We couldn't really fix the tests because it was Xcode 15 on simulators that was broken.
If the same issue remains in Xcode 16, I think we're on the same situation as before.

@harsh62 @ruisebas
Update Amplify/Core/Configuration/Internal/Amplify+Reset.swift
faaae9b 
Co-authored-by: Sebastian Villena <97059974+ruisebas@users.noreply.github.com>
ruisebas
ruisebas previously approved these changes Oct 21, 2024
View reviewed changes
@harsh62 harsh62 changed the title feat(auth): adding vision os preview feat: adding vision os preview Oct 21, 2024
@harsh62 harsh62 merged commit f950c6c into main Oct 21, 2024
93 checks passed
@harsh62 harsh62 deleted the visionos-preview-rebased branch October 21, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruisebas ruisebas ruisebas approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@harsh62 @ruisebas @atierian @thisisabhash