iOS 15: Change Buildkite image to Xcode 13 #17233

leandroalonso · 2021-09-28T16:52:47Z

This PR changes the Buildkit image to use Xcode 13. This should allow the build to stop failing.

Notice that this change is targeting a feature branch, not develop.

To test:

Make sure Buildkite build is green

Regression Notes

Potential unintended areas of impact
n/a
What I did to test those areas of impact (or what existing automated tests I relied on)
n/a
What automated tests I added (or what prevented me from doing so)
n/a

PR submission checklist:

I have completed the Regression Notes.
I have considered adding unit tests for my changes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

peril-wordpress-mobile · 2021-09-28T16:52:53Z

You can trigger an installable build for these changes by visiting CircleCI here.

leandroalonso · 2021-09-28T16:53:06Z

@mokagio I thought just changing the image to xcode-13.0 or xcode-13 would make this to work, apparently not. 😅

Could you please help me with this? Also, I'm not sure if any change should be made to CircleCI too.

mokagio · 2021-09-29T03:45:03Z

@leandroalonso hey there 👋

We are working on the Xcode 13 support on Buildkite, but it's not there yet 😞

mokagio · 2021-09-29T03:48:32Z

Also, I'm not sure if any change should be made to CircleCI too

I pushed a commit to do that. Let's see how it goes 🤞

Prior to this change, we were using `2.6.6` but that version is no longer available in the Xcode 13 image. See error message here: https://app.circleci.com/pipelines/github/wordpress-mobile/WordPress-iOS/24560/workflows/74788ee1-f42e-4876-96c6-4e75e74016d4/jobs/57379

mokagio · 2021-09-29T04:28:40Z

@leandroalonso getting CI failures due to this

❌  /Users/distiller/project/WordPress/WordPressTest/MockContext.swift:20:19: non-@objc instance method 'fetch' is declared in extension of 'NSManagedObjectContext' and cannot be overridden

It's progress, I guess... at least the 13.0 image loaded fine on CircleCI 😄

mokagio · 2021-09-29T06:50:21Z

This is a legit issue and I'm not sure how to fix it easily. We're trying to override a non open method and the compiler doesn't let us, of course. I guess this is a new or improved thing in Xcode 13?

My recommendation for this kind of things is not to subclass a framework type (NSManagedObjectContext in this instance) but to use the dependency inversion principle and dependency injection via a protocol to define an interface around it that can easily be replaced with a test double. Obviously, that would be a huge piece of work in this case 🙃

leandroalonso · 2021-09-29T12:51:24Z

~~@mokagio That's weird! I'm on Version 13.0 (13A233) and it builds just fine for me. 🙃~~

~~Have you done any additional steps? Maybe I should do a clean build.~~

Nvm, just saw that this is on the tests, shame on me. I'll look into it.

leandroalonso · 2021-09-29T17:16:25Z

@Gio2018 I looked at the failing build and it seems that MockContext is only used in the ReaderTabItemsStoreTests, you probably have a lot more context about it than I. Would you mind taking a look at it, for us?

Gio2018 · 2021-09-29T18:34:01Z

@Gio2018 I looked at the failing build and it seems that MockContext is only used in the ReaderTabItemsStoreTests, you probably have a lot more context about it than I. Would you mind taking a look at it, for us?

I might have more (mock)context 🤣 I'll take a look

… unit tests, to address build issues on Xcode 13

Gio2018 · 2021-09-29T20:51:37Z

Looks like the most sustainable thing to do at this moment is to just remove that MockContext entirely and replace it with the main context in TestContextManager. Some checks would not be allowed anymore but the general consistency of the tests should still stand. We might do a future refactor to improve things. I pushed those changes in this branch.

peril-wordpress-mobile · 2021-09-29T21:09:16Z

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

mokagio · 2021-09-30T06:19:34Z

Thank you @Gio2018. The need for CoreData improvements is on @wordpress-mobile/owl-team radar, too. It might be a good occasion to work on it.

CI is still red, but now the error is in the tests run. That is, your fix successfully sorted out the build issue. Thanks 🙌

leandroalonso · 2021-09-30T17:10:23Z

CI is still red, but now the error is in the tests run.

@mokagio is this related to fastlane and/or the new image? Shall we tackle this on this PR or open a new one?

mokagio · 2021-10-01T04:25:41Z

@leandroalonso I am not sure yet. The tests run successfully on my machine via Xcode, which seems to point out the issue has to do with CircleCI.

I updated the device and iOS version CI uses to be iPhone 12 and iOS 15.0, which work on my end.

The type of failure we're getting seems to point out to the device crashing rather than some test failing:

[04:08:57]: ▸ Loading...
[04:11:14]: ▸ 2021-10-01 04:11:14.141 xcodebuild[2774:12934] [MT] IDETestOperationsObserverDebug: 92.101 elapsed -- Testing started completed.
[04:11:14]: ▸ 2021-10-01 04:11:14.141 xcodebuild[2774:12934] [MT] IDETestOperationsObserverDebug: 0.000 sec, +0.000 sec -- start
[04:11:14]: ▸ 2021-10-01 04:11:14.141 xcodebuild[2774:12934] [MT] IDETestOperationsObserverDebug: 92.101 sec, +92.101 sec -- end
[04:11:19]: ▸ Testing failed:
[04:11:19]: ▸ 	WordPressTest:
[04:11:19]: ▸ 		WordPress (2872) encountered an error (Early unexpected exit, operation never finished bootstrapping - no restart will be attempted. (Underlying Error: Test crashed with signal abrt. If you believe this error represents a bug, please attach the result bundle at /Users/distiller/project/build/results/WordPressTest-batch-1/report.xcresult))
[04:11:19]: ▸ ** TEST EXECUTE FAILED **

Notice the "operation never finished bootstrapping".

I am now running the tests locally via Fastlane, like CI does. I'll keep you posted.

Another thing that would be useful to diagnose this problem is running Xcode 13 on Buildkite. Unfortunately the image is not ready yet. But it should be shortly.

I did this to see if it solved the issues we are experiencing with Xcode 13 testing, but that didn't help.

mokagio · 2021-10-01T05:05:46Z

Good news: running the fastlane commands from .circleci/config.yml results in the same kind of failure on my machine.

Bad news: I'm not sure how to fix it yet 😓 I also tried to update Fastlane, but that didn't help.

Here's a stacktraces I got locally:

[14:32:11]: ▸ Testing failed:
[14:32:11]: ▸ 	WordPressTest:
[14:32:11]: ▸ 		WordPress (32566) encountered an error (Early unexpected exit, operation never finished bootstrapping - no restart will be attempted. (Underlying Error: Crash: WordPress (32566) main. dyld4 config: DYLD_ROOT_PATH=/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Library/Developer/CoreSimulator/Profiles/Runtimes/iOS.simruntime/Contents/Resources/RuntimeRoot DYLD_LIBRARY_PATH=/Users/gio/Developer/a8c/wpios/DerivedData/Build/Products/Debug-iphonesimulator DYLD_INSERT_LIBRARIES=/Users/gio/Developer/a8c/wpios/DerivedData/Build/Products/Debug-iphonesimulator/WordPress.app/Frameworks/libXCTestBundleInject.dylib:/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Library/Developer/CoreSimulator/Profiles/Runtimes/iOS.simruntime/Contents/Resources/RuntimeRoot/usr/lib/libMainThreadChecker.dylib DYLD_FRAMEWORK_PATH=/Users/gio/Developer/a8c/wpios/DerivedData/Build/Products/Debug-iphonesimulator:/Users/gio/Developer/a8c/wpios/DerivedData/Build/Products/Debug-iphonesimulator/PackageFrameworks
[14:32:11]: ▸ *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '+[HTTPStubsDescriptor _identifierForSelectorString:]: unrecognized selector sent to class 0x1113e23f8'
[14:32:11]: ▸ terminating with uncaught exception of type NSException
[14:32:11]: ▸ abort() called
[14:32:11]: ▸ CoreSimulator 776.3 - Device: iPhone 13 (4E776B45-959E-45B7-B93D-6FA523EF47DB) - Runtime: iOS 15.0 (19A339) - DeviceType: iPhone 13. If you believe this error represents a bug, please attach the result bundle at /Users/gio/Developer/a8c/wpios/build/results/WordPressTest-batch-1/report.xcresult))
[14:32:11]: ▸ ** TEST EXECUTE FAILED **

I seems like the error is:

+[HTTPStubsDescriptor _identifierForSelectorString:]: unrecognized selector sent to class

I don't think the issue is with HTTPStubsDescriptor in particular because on a subsequent run, I got the same exception on a different object, TestContextManager:

[14:38:08]: ▸ *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '+[TestContextManager _identifierForSelectorString:]: unrecognized selector sent to class 0x11050cf50'

cc @jkmassel @AliSoftware for input.

leandroalonso · 2021-10-28T19:49:45Z

@mokagio I spent a few hours on that and no success yet.

A few things:

First, I tried running the commands on Xcode 13.1, same issue. :(

Secondly, there is an issue reported on the OCMock library that is similar to what we're seeing (although not equal).

In our case, tests on Xcode always succeed (at least on my tests) while they always fail on the command line.

leandroalonso · 2021-10-29T16:53:58Z

I managed to fix the execution of the tests! 🥳

This is kinda crazy and it's a mix of Xcode 13 + fastlane + naming + dependencies (phew!).

tl;dr:

The xcodebuild command generated by fastlane had some weird stuff:

"-only-testing:WordPressTest/ContextManagerMock/testExpectation" "-only-testing:WordPressTest/HTTPStubsDescriptor/testBlock" "-only-testing:WordPressTest/OCMLocation/testCase" "-only-testing:WordPressTest/TestContextManager/testExpectation"

These are not tests. It has been there since... always? But this is what was causing issues running unit tests on the command line with the Xcode 13. I submitted a change on the Fastfile to remove those.

Long story

I did a bunch of debugging and I was able to see that I was still getting crashes related to ContextManagerMock, HTTPStubsDescriptor, OCMLocation, and TestContextManager even if I just wanted to run a single simple unit test.

Then I started to debug the produced xcodebuild commands and found these odd params:

"-only-testing:WordPressTest/ContextManagerMock/testExpectation" "-only-testing:WordPressTest/HTTPStubsDescriptor/testBlock" "-only-testing:WordPressTest/OCMLocation/testCase" "-only-testing:WordPressTest/TestContextManager/testExpectation"

These are not unit tests! However, they've been there for a while (I checked some old build logs on CircleCI). Anyway, I removed them to see what happened and... the tests worked again!

I have no idea how this caused the unrecognized selector sent to class that we've been seeing. Perhaps something might have changed for Xcode 13.

Researching a little bit more I discovered that fastlane reads the xctest file and extracts everything that starts with test, and guess what? All those entities have a property or method that starts with test.

To have an elegant solution, we can look at ways to refactor some of those entities to get rid of everything that starts with test however I'm really not sure why OCMock and OHHTTPStubs stuff is being taken into account by fastlane.

So far that's all. Let me know if any of you would like any more info or clarification!

leandroalonso · 2021-10-29T18:24:37Z

@mokagio I think the PR is good to go.

If we decide to go ahead with the change on Fastfile we can add a comment there stating what that line fixes. Can I leave that to you, sir? Thanks!

mokagio · 2021-11-03T02:17:58Z

Excellent research work @leandroalonso!

You actually made me discover something I didn't know about our configuration: we don't use Fastlane's native run_test (commonly called scan) to run the tests, but a custom version called multi_scan which adds useful features such as parallelization and retries.

Turns out we are not the only folks experiencing this issue: lyndsey-ferguson/fastlane-plugin-test_center#358. The workaround suggested there is quite similar to yours!

I think it's okay to ship with your fix and to block out some time soon to work on it soon.

See #17233 (comment)

mokagio · 2021-11-03T04:30:25Z

I started investigating dropping multi_scan. Of course, that's useful to have so we can retry failed UI tests, but it's also good to run all the tests 😄

Also, Xcode 13 includes a new native retry mechanisms with the Test Repetition option which we might be able to use directly.

Change: Xcode image from 12 to 13

33746be

leandroalonso added [Type] Task iOS 15 labels Sep 28, 2021

leandroalonso added this to the Pending milestone Sep 28, 2021

leandroalonso self-assigned this Sep 28, 2021

leandroalonso requested a review from mokagio September 28, 2021 16:52

Fix: image name

bdcec49

leandroalonso marked this pull request as draft September 28, 2021 16:58

mokagio added 2 commits September 29, 2021 13:46

Bump CircleCI Xcode image version to 13.0.0

49b0ed0

Bump .xcversion value from 12.5 to 13.0

6e08748

Giorgio Ruscigno added 3 commits September 29, 2021 15:47

Remove MockContext from unit tests

52ebbd0

Rearrange project files

432ed7c

Replace the use of MockContext with TestContextManager.mainContext in…

5b15ec1

… unit tests, to address build issues on Xcode 13

Update Simulator device and iOS version to use in CircleCI

202c76a

Update Fastlane from 2.184.1 to 2.195.0

b5a75cc

I did this to see if it solved the issues we are experiencing with Xcode 13 testing, but that didn't help.

mokagio added 2 commits October 28, 2021 13:52

Use new Automattic Xcode 13 image in Buildkite

a99d8c6

Run tests on iOS 15 in Buildkite

b5418e5

leandroalonso mentioned this pull request Oct 28, 2021

Fixes the unit tests for Xcode 13. #17382

Closed

4 tasks

leandroalonso and others added 2 commits October 29, 2021 12:57

fastlane: filter tests to prevent test crashes

e337922

Fix Ruby Issue

fc376fd

leandroalonso marked this pull request as ready for review October 29, 2021 18:22

Merge branch 'issue/xcode_13' into issue/xcode_13_change_buildkite_image

518508f

mokagio added a commit that referenced this pull request Nov 3, 2021

Experiment: Use scan instead of multi_scan

444d526

See #17233 (comment)

mokagio added a commit that referenced this pull request Nov 3, 2021

Experiment: Use scan instead of multi_scan

6b55b8a

See #17233 (comment)

mokagio added a commit that referenced this pull request Nov 3, 2021

Experiment: Use scan instead of multi_scan

84aaca6

See #17233 (comment)

Add note with rationale for skipping some tests in Fastlane

e404d15

mokagio approved these changes Nov 3, 2021

View reviewed changes

leandroalonso merged commit 2d6618a into issue/xcode_13 Nov 3, 2021

leandroalonso deleted the issue/xcode_13_change_buildkite_image branch November 3, 2021 12:05

leandroalonso mentioned this pull request Nov 3, 2021

Xcode 13 support + iOS 15 fixes #17415

Merged

4 tasks

mokagio mentioned this pull request Nov 12, 2021

Fix UI tests not running since Xcode 13 migration #17456

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

iOS 15: Change Buildkite image to Xcode 13 #17233

iOS 15: Change Buildkite image to Xcode 13 #17233

leandroalonso commented Sep 28, 2021

peril-wordpress-mobile bot commented Sep 28, 2021 •

edited

Loading

leandroalonso commented Sep 28, 2021 •

edited

Loading

mokagio commented Sep 29, 2021

mokagio commented Sep 29, 2021

mokagio commented Sep 29, 2021

mokagio commented Sep 29, 2021 •

edited

Loading

leandroalonso commented Sep 29, 2021 •

edited

Loading

leandroalonso commented Sep 29, 2021

Gio2018 commented Sep 29, 2021

Gio2018 commented Sep 29, 2021

peril-wordpress-mobile bot commented Sep 29, 2021 •

edited

Loading

mokagio commented Sep 30, 2021

leandroalonso commented Sep 30, 2021

mokagio commented Oct 1, 2021

mokagio commented Oct 1, 2021 •

edited

Loading

leandroalonso commented Oct 28, 2021

leandroalonso commented Oct 29, 2021

leandroalonso commented Oct 29, 2021 •

edited

Loading

mokagio commented Nov 3, 2021

mokagio commented Nov 3, 2021 •

edited

Loading

iOS 15: Change Buildkite image to Xcode 13 #17233

iOS 15: Change Buildkite image to Xcode 13 #17233

Conversation

leandroalonso commented Sep 28, 2021

To test:

Regression Notes

peril-wordpress-mobile bot commented Sep 28, 2021 • edited Loading

leandroalonso commented Sep 28, 2021 • edited Loading

mokagio commented Sep 29, 2021

mokagio commented Sep 29, 2021

mokagio commented Sep 29, 2021

mokagio commented Sep 29, 2021 • edited Loading

leandroalonso commented Sep 29, 2021 • edited Loading

leandroalonso commented Sep 29, 2021

Gio2018 commented Sep 29, 2021

Gio2018 commented Sep 29, 2021

peril-wordpress-mobile bot commented Sep 29, 2021 • edited Loading

mokagio commented Sep 30, 2021

leandroalonso commented Sep 30, 2021

mokagio commented Oct 1, 2021

mokagio commented Oct 1, 2021 • edited Loading

leandroalonso commented Oct 28, 2021

leandroalonso commented Oct 29, 2021

tl;dr:

Long story

leandroalonso commented Oct 29, 2021 • edited Loading

mokagio commented Nov 3, 2021

mokagio commented Nov 3, 2021 • edited Loading

peril-wordpress-mobile bot commented Sep 28, 2021 •

edited

Loading

leandroalonso commented Sep 28, 2021 •

edited

Loading

mokagio commented Sep 29, 2021 •

edited

Loading

leandroalonso commented Sep 29, 2021 •

edited

Loading

peril-wordpress-mobile bot commented Sep 29, 2021 •

edited

Loading

mokagio commented Oct 1, 2021 •

edited

Loading

leandroalonso commented Oct 29, 2021 •

edited

Loading

mokagio commented Nov 3, 2021 •

edited

Loading