Run Xcode 15 in CI and skip or disable tests that fail #21921

mokagio · 2023-10-31T04:46:24Z

A bit of an underwhelming PR, where instead of fixing things, I work around failures by disabling tests 😞 In my defense, there doesn't seem much we can do about how the existing implementation of the problematic tests behaves with the iOS 17 SDK.

I thought I'd put this up as a starting point. The unit tests passed in the draft PR. Merging would at least let us officially use Xcode 15.0.1 as development toolchain, so that we can move on to fixing things and find new issues to tackle.

Open to suggestions on how to update the failing code.

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. N.A.
I have considered adding accessibility improvements for my changes. N.A.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary. N.A.

UI changes testing checklist: Not a UI PR.

wpmobilebot · 2023-10-31T05:11:19Z

📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.

	App Name	WordPress Alpha
	Configuration	Release-Alpha
	Build Number	pr21921-692dace
	Version	23.9
	Bundle ID	org.wordpress.alpha
	Commit	`692dace`
	App Center Build	WPiOS - One-Offs #8128

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

wpmobilebot · 2023-10-31T05:19:44Z

📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.

	App Name	Jetpack Alpha
	Configuration	Release-Alpha
	Build Number	pr21921-692dace
	Version	23.9
	Bundle ID	com.jetpack.alpha
	Commit	`692dace`
	App Center Build	jetpack-installable-builds #7148

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

kean · 2023-10-31T13:46:50Z

I found the following issue when testing on Xcode 15 with the new iOS SDK: #21649 (blocker). There may be more issues of a similar kind. I'll require a bit more effort to migrate to Xcode 15 because it also means migrating to iOS 17, which the app currently doesn't (fully) support.

mokagio · 2023-10-31T20:50:16Z

Thanks @kean 👍

I subscribed to the issue. I also created an Xcode 15 and iOS 17 project in this repo as a quick way to collect and monitor progress on this kind of issues. Feel free to add to it, thanks!

See fastlane/fastlane#21493

mokagio · 2023-11-20T00:26:01Z

WordPress/WordPressTest/WKCookieJarTests.swift

-    var wkCookieStore = WKWebsiteDataStore.nonPersistent().httpCookieStore
+    var wkCookieStore: WKHTTPCookieStore!


The value is being assigned in setUp. It's best to do it like that anyway, so we get a fresh one for every test.

kean · 2023-11-20T19:31:18Z

LGTM.

The fix for post scheduling in iOS 17 was also fixed #22012. I've been running Xcode 15 for a couple of weeks now with no issues. I also need Xcode 15 for the following PR #22060, so I would appreciate if we could get this into trunk sooner rather than later.

mokagio · 2023-11-21T01:10:02Z

Thanks @pachlava for having a look at the UI tests! 🙌

I'll share here a summary of an internal conversation for visibility: Sergiy noticed that the UI tests are super slow and, because of the slowness, more flaky and prone to fail. (Talk about selective attention, I looked at every single CI build but was only focused on the unit tests and was totally oblivious to how long the UI tests took!) The UI tests alone can now take more than 2 hours to run, that's too long.

Unfortunately, this is a known performance issue with Xcode 15 in CI for which there doesn't seem to be a fix yet.

Given the performance implications, I see only two options:

Disable the UI tests, which we don't want to do because we care about what they tell us; or
Delay adopting Xcode 15, which we don't want to do because of the need to address iOS 17 issues

As much as it pains me, I think we might need to disable the UI tests in CI. Nothing should stand in the way of fixing user issues.

But before doing that, it is also possible that our new M1 CI infra will help with this, based on some of the discussion in the link above and on this forums thread Apple Silicon hardware fairs better. That's not surprising, of course, in terms of raw performance, but there might be something structural going on there, too. cc @jkmassel

kean · 2023-11-21T01:44:38Z

I'll add one more potential option for consideration: run UI tests once a day and deliver a report to the team in case they fail. This will alleviate 50%+ of the pressure from the CI. If it's currently feasible to run UI tests for every build, it means there are not enough of them.

jkmassel · 2023-11-21T06:27:39Z

I'd like to see how this performs on Apple Silicon – it's great that this PR targets 15.0.1, because it means that we're immediately compatible and can easily switch back and forth.

I'd be surprised if the performance issues hit us too badly there, from the thread it seems like it's mostly folks who are using Rosetta to run their tests, but I didn't read in-depth. If the tests continue to take an unacceptably long amount of time we can subdivide them further – UI tests are pretty parallelizable.

If none of that works, I'd really like to dig into the tests and find out exactly what they're doing, and if this behaviour can be replicated locally – maybe it's something to do with running under a VM? We've seen enormous instability with Xcode 15.0 for Tumblr, so the idea that performance on Intel is somehow compromised in that version isn't too surprising. The good news is that Apple Silicon made an enormous difference there, so hopefully it will here too?

I'd very much like to avoid turning off a test suite unless we're left with no other option – and running it on some kind of schedule is essentially turning it off. Every time we've tried it we've ended up in a situation where the tests are broken the majority of the time, so even legitimate failures are dismissed as flakiness. If we can't run a test on every PR, that test isn't worth having. And if we need more capacity to run those tests, we'll add it – computers are cheap, people's time is expensive, so we run the tests – as often as we can.

pachlava · 2023-11-21T10:59:06Z

run UI tests once a day and deliver a report to the team in case they fail.

@kean 👋 The issue with UI tests after upgrade to Xcode 15 is not the slowness alone, it's the fact that slowness causes timeouts, resulting in them always failing. One alternative is running them locally against trunk daily, which is probably better than nothing.

kean · 2023-11-21T11:06:12Z

The current UI test suite has only 35 tests, which is not a lot of tests tbh, and it already overloads the CI. From the developer side, if it takes more than 10 minutes, I'd say it shouldn't run on every push or it will hinder the development flow and works against short-lived branches and TBD.

The good news is that Apple Silicon made an enormous difference there, so hopefully it will hear too?

How long does it take to run the tests on M2/M3 locally? I've yet to run the tests locally because you need a Java runtime. I begrudgingly installed it, ran rake mocks, turned off the proxy, but I am still yet to be able to run them 😞

daily, which is probably better than nothing.

That's how it's traditionally been done, otherwise, it sets the wrong incentives. The incentive should be to allow the automation team to create as many UI tests as possible and not worry about their performance as long as it takes no more than 8-12 hours to run the entire suite.

jostnes · 2023-11-27T05:49:17Z

@jostnes, I had to change the comment text locator in 655a424, because for whatever reason, on CI the comment text is not discoverable at all now.

Thank you for making the change @pachlava! Also I agree with you that we could probably stop running the tests on iPad since it's extremely slow and tests keep failing because of time-outs, and keep it iPhone only for the time being. But I also wonder if this slowness is only seen on CI or will it be seen by actual users on a slower network as well after this change?

pachlava · 2023-11-27T08:50:29Z

@jostnes, from my understanding of the issue, this is related to virtualization that is used on CI (affects both BK and CircleCI). It's not related to the network speed.

…ess-mobile/WordPress-iOS into mokagio/fix-code-15-tests

mokagio · 2023-12-15T09:24:33Z

WordPress/UITestsFoundation/Screens/Login/Unified/PasswordScreen.swift

@@ -29,7 +29,7 @@ public class PasswordScreen: ScreenObject {
    @discardableResult
    public func proceedWithValidPassword() throws -> LoginEpilogueScreen {
        try tryProceed(password: "pw")
-
+        app.dismissSavePasswordPrompt()


@jostnes I'm surprised by this. tryProceed(password:) already calls it.

Did it make a difference or is it an experiment?

oh yeah, this has been removed, i overlooked this earlier, this was also the reason the test took a longer time to complete (as it was doing it twice, the second time completely unnecessary 😅)

mokagio · 2023-12-15T09:37:10Z

WordPress/UITests/JetpackUITests-Info.plist

@@ -20,7 +20,5 @@
 	<string>????</string>
 	<key>CFBundleVersion</key>
 	<string>1</string>
-	<key>NSPrincipalClass</key>
-	<string>${PRODUCT_NAME}.TestObserver</string>


@jostnes I'd recommend keeping this setup and skipping the executeWithRetries(disableAutoFillPasswords) in testBundleWillStart from TestObserver.

I'm saying this because it's easier to find the TestObserver setup in the code than in this Info.plist and hopefully we'll be able to go back to that setup when Apple fixes their Simulator bug (assuming my theory about it being a simulator bug is correct)

yup, this is a temporary removal to save testing time and skip the part where the test tries to turn it off and fails every time because of the simulator bug. this will be re-added before merging.

mokagio · 2024-01-03T02:42:24Z

Closing in favor of @jostnes' #22270

mokagio self-assigned this Oct 31, 2023

mokagio added the Testing Unit and UI Tests and Tooling label Oct 31, 2023

mokagio added 7 commits November 20, 2023 09:59

Use Xcode 15.0.1 in CI

4fae8c7

Update Fastlane to version 2.217.0 to get Xcode 15 trainer fix

e5ea649

See fastlane/fastlane#21493

Update TimeZoneFormatterTests to pass on Xcode 15

d9ca84f

Remove a useless reassignment in WKCookieJarTests

0404e96

Mark WKCookieJarTests as expected to fail

0eb5fca

Remove redundant custom test assertion, which was also out of sync

b57be1b

Skip two tests failing because of a changed WKWebView behavior

615bffb

mokagio force-pushed the mokagio/fix-code-15-tests branch from 2dec694 to 615bffb Compare November 19, 2023 23:03

mokagio commented Nov 20, 2023

View reviewed changes

mokagio changed the title ~~Run Xcode 15 in CI~~ Run Xcode 15 in CI and skip or disable tests that fail Nov 20, 2023

mokagio marked this pull request as ready for review November 20, 2023 00:26

mokagio requested review from crazytonyli and kean November 20, 2023 00:30

mokagio added this to the 23.8 milestone Nov 20, 2023

[UI Tests] Replaced "4 AM" with "4 AM".

16e9b7a

kean approved these changes Nov 20, 2023

View reviewed changes

mokagio enabled auto-merge November 21, 2023 00:42

mokagio disabled auto-merge November 21, 2023 00:59

mokagio modified the milestones: 23.8, 23.9 Nov 27, 2023

pachlava mentioned this pull request Dec 5, 2023

Update Xcode version from 14.3.1 to 15.1 on CI woocommerce/woocommerce-ios#11352

Merged

3 tasks

mokagio modified the milestones: 23.9, 24.0 Dec 10, 2023

mokagio and others added 9 commits December 14, 2023 11:41

Merge 'origin/trunk' into mokagio/fix-code-15-tests

692dace

Use Xcode 15.1

9f744d0

Fix indentation in TestObserver

c233e83

Merge branch 'trunk' into mokagio/fix-code-15-tests

779b70d

Add workaround for iOS 17.2 Simulator breaking passwords settings

b328aec

temp remove TestObserver

51b50aa

temp remove auto retries

f7ae151

added workarounds around login

4e0ef37

Merge branch 'mokagio/fix-code-15-tests' of https://github.com/wordpr…

ebff3e7

…ess-mobile/WordPress-iOS into mokagio/fix-code-15-tests

mokagio commented Dec 15, 2023

View reviewed changes

jostnes added 6 commits December 18, 2023 11:55

decrease timeout

85f8c5d

remove unnecessary checks

fd5c800

fix logic for tapUntilCondition

67e3c73

turn retry back on

257e78f

fix stats test by making year dynamic

d8ec5d2

add retry to dismiss save password prompt

4c3ca35

jostnes mentioned this pull request Dec 22, 2023

Test Fixes to Run Xcode 15 in CI #22270

Merged

4 tasks

guarani mentioned this pull request Jan 2, 2024

Fix "Please adopt containerBackgroundAPI" for iOS 17 Widgets #21705

Merged

13 tasks

mokagio closed this Jan 3, 2024

mokagio mentioned this pull request Jan 3, 2024

test_fetchPrompts_passesTheDateCorrectly fails semi-consistently #22323

Closed

jkmassel deleted the mokagio/fix-code-15-tests branch July 26, 2024 18:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run Xcode 15 in CI and skip or disable tests that fail #21921

Run Xcode 15 in CI and skip or disable tests that fail #21921

mokagio commented Oct 31, 2023 •

edited

Loading

wpmobilebot commented Oct 31, 2023 •

edited

Loading

wpmobilebot commented Oct 31, 2023 •

edited

Loading

kean commented Oct 31, 2023 •

edited

Loading

mokagio commented Oct 31, 2023

mokagio Nov 20, 2023

kean commented Nov 20, 2023 •

edited

Loading

mokagio commented Nov 21, 2023 •

edited

Loading

kean commented Nov 21, 2023 •

edited

Loading

jkmassel commented Nov 21, 2023

pachlava commented Nov 21, 2023

kean commented Nov 21, 2023 •

edited

Loading

jostnes commented Nov 27, 2023

pachlava commented Nov 27, 2023

mokagio Dec 15, 2023

jostnes Dec 18, 2023

mokagio Dec 15, 2023

jostnes Dec 18, 2023

mokagio commented Jan 3, 2024

		var wkCookieStore = WKWebsiteDataStore.nonPersistent().httpCookieStore
		var wkCookieStore: WKHTTPCookieStore!

Run Xcode 15 in CI and skip or disable tests that fail #21921

Run Xcode 15 in CI and skip or disable tests that fail #21921

Conversation

mokagio commented Oct 31, 2023 • edited Loading

Regression Notes

wpmobilebot commented Oct 31, 2023 • edited Loading

wpmobilebot commented Oct 31, 2023 • edited Loading

kean commented Oct 31, 2023 • edited Loading

mokagio commented Oct 31, 2023

mokagio Nov 20, 2023

Choose a reason for hiding this comment

kean commented Nov 20, 2023 • edited Loading

mokagio commented Nov 21, 2023 • edited Loading

kean commented Nov 21, 2023 • edited Loading

jkmassel commented Nov 21, 2023

pachlava commented Nov 21, 2023

kean commented Nov 21, 2023 • edited Loading

jostnes commented Nov 27, 2023

pachlava commented Nov 27, 2023

mokagio Dec 15, 2023

Choose a reason for hiding this comment

jostnes Dec 18, 2023

Choose a reason for hiding this comment

mokagio Dec 15, 2023

Choose a reason for hiding this comment

jostnes Dec 18, 2023

Choose a reason for hiding this comment

mokagio commented Jan 3, 2024

mokagio commented Oct 31, 2023 •

edited

Loading

wpmobilebot commented Oct 31, 2023 •

edited

Loading

wpmobilebot commented Oct 31, 2023 •

edited

Loading

kean commented Oct 31, 2023 •

edited

Loading

kean commented Nov 20, 2023 •

edited

Loading

mokagio commented Nov 21, 2023 •

edited

Loading

kean commented Nov 21, 2023 •

edited

Loading

kean commented Nov 21, 2023 •

edited

Loading