Address flaky Alamofire network test on Alamofire.AFError.sessionDeinitialized error from concurrent requests

[jaclync]

For WOOMOB-1326

Just one review is required.

Description

This PR addresses a flaky test in the Alamofire networking layer that were causing intermittent test failures due to race conditions. It also updates a similar test case to not make an external network request. The changes address two main issues:

1. Removed flaky concurrent request test: The test AlamofireNetworkTests.test_concurrent_requests_fail_with_sessionDeinitialized_error_when_ensuresSessionManagerIsInitialized_is_false was removed because it relied on a race condition between Alamofire.AFError.sessionDeinitialized and network errors that could be thrown simultaneously, making the test unreliable.

• I tried a few approaches like updating the concurrent requests to a URL that never returns, or a localhost address with a quick timeout, but the life cycle of the Alamofire.Session is unpredictable and Alamofire.AFError.sessionDeinitialized isn't consistently thrown right away.
• I also tried replacing the alamofireSession: Alamofire.Session from a lazy var to a let, but it results in a crash when running test_authenticationMode_changes_to_appPasswordsWithJetpack_when_app_password_switching_enabled on this line that updates the fixed RequestProcessor:

NetworkingCore`async function pointer to NetworkingCore.AlamofireNetwork.responseDataAndHeaders(for: Alamofire.URLRequestConvertible) async throws -> (Foundation.Data, Swift.Optional<Swift.Dictionary<Swift.String, Swift.String>>):
->  0x10e55f410 <+0>: .long  0xfff70450                ; unknown opcode
    0x10e55f414 <+4>: udf    #0x160

But I ran out of time to look further, and didn't have enough context of the recent authentication changes from Kiwi to determine the cause. It felt a bit concerning that the test case is relying on the session being a lazy variable, even though that test case actually DI'ed a mock session. It might just be the test setup, though.

3. Updated concurrent request test to use internal network call: The remaining test AlamofireNetworkTests.test_concurrent_requests_do_not_fail_with_sessionDeinitialized_error_when_ensuresSessionManagerIsInitialized_is_true now uses a direct URLRequest to localhost with a very short timeout instead of a JetpackRequest, making it more predictable and eliminating the race condition.

Steps to reproduce

CI passing is sufficient, but feel free to run AlamofireNetworkTests repeatedly locally.

Testing information

I ran AlamofireNetworkTests repeatedly (100 times) multiple times locally and esnured that they passed. I was able to reproduce the flakiness by running AlamofireNetworkTests repeatedly (100 times) once before the PR changes.

---

• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

[wpmobilebot]

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

	App Name	WooCommerce iOS Prototype
	Build Number	pr16137-05c3061
	Version	23.3
	Bundle ID	com.automattic.alpha.woocommerce
	Commit	05c3061
	Installation URL	3jdh9qqbkfnmo

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

[iangmaia]

Version 23.3 has now entered code-freeze, so the milestone of this PR has been updated to 23.4.

[itsmeichigo]

Regarding this part:

I also tried replacing the alamofireSession: Alamofire.Session from a lazy var to a let, but it results in a crash when running test_authenticationMode_changes_to_appPasswordsWithJetpack_when_app_password_switching_enabled on this line that updates the fixed RequestProcessor

This line is no longer in trunk after the changes from last week. Do you think it's worth trying your approach again to update the test_concurrent_requests_fail_with_sessionDeinitialized_error_when_ensuresSessionManagerIsInitialized_is_false test?

[jaclync]

Regarding this part:

I also tried replacing the alamofireSession: Alamofire.Session from a lazy var to a let, but it results in a crash when running test_authenticationMode_changes_to_appPasswordsWithJetpack_when_app_password_switching_enabled on this line that updates the fixed RequestProcessor

This line is no longer in trunk after the changes from last week. Do you think it's worth trying your approach again to update the test_concurrent_requests_fail_with_sessionDeinitialized_error_when_ensuresSessionManagerIsInitialized_is_false test?

Nice, thanks for sharing the update. I replaced the lazy var Alamofire.Session with a let in e1c5a7d. Unit tests passing locally and on CI, the PR is ready for another look.

[itsmeichigo]

Thanks for the update! The tests pass locally for me too. I also did some regression tests and confirmed that network switching still works correctly after the change to alamofireSession.

[jaclync]

Thanks for the review & additional testing @itsmeichigo, I also tested again for concurrent requests in POS catalog sync for stores with big & small catalog size. Merging it now.