WordPress-iOS icon indicating copy to clipboard operation
WordPress-iOS copied to clipboard

A couple workarounds to make the app compiles on Xcode 14 beta

Open crazytonyli opened this issue 3 years ago • 6 comments

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

Changes

This PR fixes a couple compiling issues on Xcode 14 beta 4.

  • The Charts library doesn't compile on Xcode 14 Solution: switch to a fork of the Charts library. This change is meant to be temporary, which shouldn't land on the trunk branch. There are a couple PR open in the official repository to address this. I'll keep an eye on the repository and revert this change once the official library supports Xcode 14.
  • 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.

crazytonyli avatar Jul 28 '22 03:07 crazytonyli

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 avatar Jul 29 '22 04:07 wpmobilebot

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.

wpmobilebot avatar Jul 29 '22 05:07 wpmobilebot

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 avatar Aug 01 '22 21:08 crazytonyli

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.

crazytonyli avatar Aug 02 '22 01:08 crazytonyli

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:

https://github.com/wordpress-mobile/WordPress-iOS/blob/6fb314584170dd2817936c4835b178f8a4e55904/fastlane/lanes/build.rb#L71-L74

https://github.com/wordpress-mobile/WordPress-iOS/blob/6fb314584170dd2817936c4835b178f8a4e55904/.buildkite/commands/run-ui-tests.sh#L31

https://github.com/wordpress-mobile/WordPress-iOS/blob/6fb314584170dd2817936c4835b178f8a4e55904/.buildkite/pipeline.yml#L79

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

mokagio avatar Aug 03 '22 04:08 mokagio

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 avatar Aug 10 '22 08:08 crazytonyli

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.

crazytonyli avatar Aug 30 '22 01:08 crazytonyli

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.

crazytonyli avatar Sep 13 '22 23:09 crazytonyli