Make the app compatible with Xcode 14 #19119

crazytonyli · 2022-07-28T03:19:24Z

Please note this PR goes to the xcode-14-support branch.

Changes

This PR fixes a couple compiling issues on Xcode 14

In UI test, action sheet UI elements no longer get assigned the "sheet" element type, which causes the app.sheets query to return nothing.
Solution: remove the .sheets query.
Some generated intent code produces warnings. This is a known issue in Xcode beta.
Solution: turn off "Treat warnings as errors" in a couple targets. I'll keep an eye on these warnings and revert this change once this known issue is fixed.

Test Instructions

No test required, as this PR goes to xcode-14-support branch.

wpmobilebot · 2022-07-29T03:51:30Z

Podfile

+  # This solution is suggested here: https://github.com/CocoaPods/CocoaPods/issues/11402#issuecomment-1189861270
+  # ====================================
+  #
+  installer.pods_project.targets.each do |target|


🚫 Combine this loop with the previous loop.

It's concerning that Danger didn't remove this comment after the rubocop:disable annotation was added.

wpmobilebot · 2022-07-29T04:59:01Z

You can test the changes in WordPress from this Pull Request by:

Clicking here or scanning the QR code below to access App Center
Then installing the build number pr19119-169da2d on your iPhone

If you need access to App Center, please ask a maintainer to add you.

wpmobilebot · 2022-07-29T05:02:08Z

You can test the changes in Jetpack from this Pull Request by:

Clicking here or scanning the QR code below to access App Center
Then installing the build number pr19119-169da2d on your iPhone

If you need access to App Center, please ask a maintainer to add you.

crazytonyli · 2022-08-01T10:34:06Z

WordPress/UITestsFoundation/Screens/Editor/BlockEditorScreen.swift

-                    .selectAlbum(atIndex: 0)
+        if #available(iOS 16.0, *), XCUIDevice.isPhone {
+            // After the UI interuption monitor handler is called, the "Recents" album is automatically tapped.
+            // But this only happens on iPhone, not iPad...


It turns out, this "automatic album selection" is device specific. For example, it occurs on iPhone 13 mini, but not iPhone SE, not sure about other simulators. This now looks very much like a bug in Xcode beta, I'm thinking removing this code, and let affected tests fail (for now), and re-evaluate in next beta.

crazytonyli · 2022-08-01T21:59:46Z

I'll convert this PR to draft and re-evaluate once next beta is out, since the behavior I noted above is too weird to be a legit change.

crazytonyli · 2022-08-02T01:38:04Z

I think I'll stop trying to fix these test failures and re-evaluate them in next beta, but I'll summarize their status:

It appears CoreData's built-in data migration changed, which causes the failure of the testMigrate19to21Failureunit test. I've asked in Slack to see if this test can be deleted since it doesn't test our code.
UI tests may pass or fail, depending on the simulator model. On my mac they pass on iPhone 13 mini and iPad mini, but not on iPhone SE (not sure about others). Since we don't specific simulator device model for UI tests, UI tests on CI are very likely to fail.

mokagio · 2022-08-03T04:35:36Z

WordPress/UITestsFoundation/Screens/MySiteScreen.swift

@@ -84,7 +84,7 @@ public class MySiteScreen: ScreenObject {
        if XCUIDevice.isPad {
            removeButton = app.alerts.buttons.element(boundBy: 1)
        } else {
-            removeButton = app.sheets.buttons.element(boundBy: 0)
+            removeButton = app.buttons["Remove Site"]


Out of scope for this PR, but it would be great if we could have accessibility identifiers for this and the "Remove" used above. cc @wordpress-mobile/mobile-ui-testing-squad

mokagio · 2022-08-03T04:41:42Z

Thank you for looking into these @crazytonyli

I think I'll stop trying to fix these test failures and re-evaluate them in next beta

Fair.

For what concerns the test failures, we could marked them with XCTExpectFailure. I'm suggesting this because it would be good to see a "green" CI to make sure no other issue is present.

Since we don't specific simulator device model for UI tests, UI tests on CI are very likely to fail.

We should be specifying the device. See:

WordPress-iOS/fastlane/lanes/build.rb

Lines 71 to 74 in 6fb3145

    
           run_tests( 
        
             workspace: WORKSPACE_PATH, 
        
             scheme: 'WordPress', 
        
             device: options[:device],

WordPress-iOS/.buildkite/commands/run-ui-tests.sh

Line 31 in 6fb3145

    
           bundle exec fastlane test_without_building name:"$TEST_NAME" try_count:3 device:"$DEVICE" ios_version:"$IOS_VERSION"

WordPress-iOS/.buildkite/pipeline.yml

Line 79 in 6fb3145

command: .buildkite/commands/run-ui-tests.sh WordPressUITests 'iPhone 13' 15.0

If that device is not picked up when the tests run, then there's some issue in our tooling / Fastlane / the beta xcodebuild.

crazytonyli · 2022-08-10T08:30:48Z

UI tests may pass or fail, depending on the simulator model. On my mac they pass on iPhone 13 mini and iPad mini, but not on iPhone SE (not sure about others). Since we don't specific simulator device model for UI tests, UI tests on CI are very likely to fail.

This issue has been fixed in beta 5. UI tests passed using iPhone 13, iPhone SE, and iPad mini on my Mac.

crazytonyli · 2022-08-30T01:59:37Z

All issues discovered so far have been fixed in Xcode 14 beta 6. I was hoping the change where action sheets aren't assigned a "Sheet" element type in UI test would be fixed in new betas, I doubt it's going to happen now.

— WordPressIntents and WordPressStatsWidgets. This is a known issue documented in Xcode's release notes: > Generated Swift code that the Intent definition compiler produces > emits a warning when compiled: Hopefully this issue will be addressed in future betas, and then we can revert this commit.

In Xcode 14 beta 4, the element type of an action sheet is 'Other', rather than 'Sheet', which makes the `sheets` query fails. This might be a bug in Xcode beta.

crazytonyli · 2022-09-13T23:54:41Z

Hi @mokagio , this PR is finally ready for review. I've cleaned up the commit history and the changes in this PR now only contains what are necessary to be compatible with Xcode 14.

mokagio · 2022-09-14T03:32:36Z

Podfile

+  # This solution is suggested here: https://github.com/CocoaPods/CocoaPods/issues/11402#issuecomment-1189861270
+  # ====================================
+  #
+  installer.pods_project.targets.each do |target|


It's concerning that Danger didn't remove this comment after the rubocop:disable annotation was added.

mokagio · 2022-09-14T03:34:53Z

WordPress/WordPress.xcodeproj/project.pbxproj

@@ -23036,6 +23036,7 @@
 				SWIFT_ACTIVE_COMPILATION_CONDITIONS = DEBUG;
 				SWIFT_OBJC_BRIDGING_HEADER = "WordPressStatsWidgets/Supporting Files/WordPressStatsWidgets-Bridging-Header.h";
 				SWIFT_OPTIMIZATION_LEVEL = "-Onone";
+				SWIFT_TREAT_WARNINGS_AS_ERRORS = NO;


Sad it had to come to this in c7ffd64.

Hopefully we can remember to check this when a new beta comes.

Actually... What do you think about tracking known workaround like this one in a checklist somewhere, so that we can run it when a new beta is release?

Sounds like a good idea. Any suggestions where to track this kind of issue?

Would be helpful to know what made us unable to turn that back on.

Like, for the case where Swift Bridging Header generated by Xcode contained warnings (due to some CoreData methods, iirm), we ended up finding a solution by trampolining to a header that added push/pop diagnostics pragma around it, and that worked great. So maybe for whatever case we had there that once again prevented us from turning it on there's also a way?

I had looked for some Swift compiler flags to suppress this particular warning, but I couldn't find any. I don't think Swift compiler support diagnosis pragma directive in clang (or has anything similar to it).

But I just noticed the warnings has gone away in Xcode 14. I've opened #19310 to revert this change.

crazytonyli requested a review from a team July 28, 2022 03:19

crazytonyli force-pushed the xcode-14/workarounds branch from 07d45f7 to 544118b Compare July 28, 2022 03:26

crazytonyli added this to the Someday milestone Jul 28, 2022

crazytonyli self-assigned this Jul 28, 2022

crazytonyli changed the title ~~Xcode 14/workarounds~~ A couple workarounds to make the app compiles on Xcode 14 beta Jul 28, 2022

crazytonyli added the Tooling Build, Release, and Validation Tools label Jul 28, 2022

wpmobilebot reviewed Jul 29, 2022

View reviewed changes

crazytonyli commented Aug 1, 2022

View reviewed changes

crazytonyli marked this pull request as draft August 1, 2022 21:59

crazytonyli mentioned this pull request Aug 2, 2022

Make the app compatible with Xcode 14 woocommerce/woocommerce-ios#7371

Merged

mokagio reviewed Aug 3, 2022

View reviewed changes

crazytonyli force-pushed the xcode-14/workarounds branch from 5ae7a65 to cd1db7d Compare September 1, 2022 04:53

crazytonyli added 5 commits September 14, 2022 11:44

Disable a linting issue in Podfile for now

07b1cd4

Fix looking up buttons in action sheets in UI test

c4a5009

In Xcode 14 beta 4, the element type of an action sheet is 'Other', rather than 'Sheet', which makes the `sheets` query fails. This might be a bug in Xcode beta.

Use iPhone SE to run UI tests

9acbbf6

Use Xcode 14 on CI

169da2d

crazytonyli force-pushed the xcode-14/workarounds branch from db05f2e to 169da2d Compare September 13, 2022 23:46

crazytonyli marked this pull request as ready for review September 13, 2022 23:52

crazytonyli requested a review from a team as a code owner September 13, 2022 23:52

crazytonyli changed the title ~~A couple workarounds to make the app compiles on Xcode 14 beta~~ Make the app compatible with Xcode 14 Sep 13, 2022

mokagio approved these changes Sep 14, 2022

View reviewed changes

crazytonyli merged commit fbc9285 into xcode-14-support Sep 14, 2022

crazytonyli deleted the xcode-14/workarounds branch September 14, 2022 06:34

ipalm0423 mentioned this pull request Sep 21, 2022

Fix push prompt button overlapping on iPad #19304

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make the app compatible with Xcode 14 #19119

Make the app compatible with Xcode 14 #19119

crazytonyli commented Jul 28, 2022 •

edited

Loading

wpmobilebot Jul 29, 2022

mokagio Sep 14, 2022

wpmobilebot commented Jul 29, 2022 •

edited

Loading

wpmobilebot commented Jul 29, 2022 •

edited

Loading

crazytonyli Aug 1, 2022

crazytonyli commented Aug 1, 2022

crazytonyli commented Aug 2, 2022 •

edited

Loading

mokagio Aug 3, 2022

mokagio commented Aug 3, 2022

crazytonyli commented Aug 10, 2022

crazytonyli commented Aug 30, 2022

crazytonyli commented Sep 13, 2022

mokagio Sep 14, 2022

mokagio Sep 14, 2022

crazytonyli Sep 14, 2022

AliSoftware Sep 14, 2022

crazytonyli Sep 14, 2022

Make the app compatible with Xcode 14 #19119

Make the app compatible with Xcode 14 #19119

Conversation

crazytonyli commented Jul 28, 2022 • edited Loading

Changes

Test Instructions

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wpmobilebot commented Jul 29, 2022 • edited Loading

wpmobilebot commented Jul 29, 2022 • edited Loading

Choose a reason for hiding this comment

crazytonyli commented Aug 1, 2022

crazytonyli commented Aug 2, 2022 • edited Loading

Choose a reason for hiding this comment

mokagio commented Aug 3, 2022

crazytonyli commented Aug 10, 2022

crazytonyli commented Aug 30, 2022

crazytonyli commented Sep 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crazytonyli commented Jul 28, 2022 •

edited

Loading

wpmobilebot commented Jul 29, 2022 •

edited

Loading

wpmobilebot commented Jul 29, 2022 •

edited

Loading

crazytonyli commented Aug 2, 2022 •

edited

Loading