From 9703c8da2080f1614813f23eb1788120cd610a88 Mon Sep 17 00:00:00 2001 From: Caio Zullo Date: Thu, 28 Sep 2023 13:49:08 +0300 Subject: [PATCH] iOS17+ fix: Move data refresh from `viewDidLoad` to new ViewController's `viewIsAppearing` lifecycle method - This change is needed since UIRefreshControl updates must happen when the view is on-screen on iOS17+ (it ignores updates otherwise!). - `viewDidLoad` is called once per VC, but the new `viewIsAppearing` may be called multiple times - so we added logic to ensure the refresh only happens in the first `viewIsAppearing` occurrence to mimic the previous behavior in `viewDidLoad`. - When running tests, the views are never added on-screen, so UIRefreshControl doesn't work as expected on iOS 17+. To allow testability, we replace the UIRefreshControl with a UIRefreshControlSpy test-double. --- .../CommentsUIIntegrationTests.swift | 37 ++++++--- .../FeedAcceptanceTests.swift | 8 +- .../FeedUIIntegrationTests.swift | 81 +++++++++++-------- .../ListViewController+TestHelpers.swift | 45 ++++++++++- .../Controllers/ListViewController.swift | 14 +++- 5 files changed, 131 insertions(+), 54 deletions(-) diff --git a/EssentialApp/EssentialAppTests/CommentsUIIntegrationTests.swift b/EssentialApp/EssentialAppTests/CommentsUIIntegrationTests.swift index a966341e..848c8622 100644 --- a/EssentialApp/EssentialAppTests/CommentsUIIntegrationTests.swift +++ b/EssentialApp/EssentialAppTests/CommentsUIIntegrationTests.swift @@ -14,17 +14,17 @@ class CommentsUIIntegrationTests: XCTestCase { func test_commentsView_hasTitle() { let (sut, _) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() XCTAssertEqual(sut.title, commentsTitle) } func test_loadCommentsActions_requestCommentsFromLoader() { let (sut, loader) = makeSUT() - XCTAssertEqual(loader.loadCommentsCallCount, 0, "Expected no loading requests before view is loaded") + XCTAssertEqual(loader.loadCommentsCallCount, 0, "Expected no loading requests before view appears") - sut.loadViewIfNeeded() - XCTAssertEqual(loader.loadCommentsCallCount, 1, "Expected a loading request once view is loaded") + sut.simulateAppearance() + XCTAssertEqual(loader.loadCommentsCallCount, 1, "Expected a loading request once view appears") sut.simulateUserInitiatedReload() XCTAssertEqual(loader.loadCommentsCallCount, 1, "Expected no request until previous completes") @@ -38,11 +38,22 @@ class CommentsUIIntegrationTests: XCTestCase { XCTAssertEqual(loader.loadCommentsCallCount, 3, "Expected yet another loading request once user initiates another reload") } + func test_loadCommentsActions_runsAutomaticallyOnlyOnFirstAppearance() { + let (sut, loader) = makeSUT() + XCTAssertEqual(loader.loadCommentsCallCount, 0, "Expected no loading requests before view appears") + + sut.simulateAppearance() + XCTAssertEqual(loader.loadCommentsCallCount, 1, "Expected a loading request once view appears") + + sut.simulateAppearance() + XCTAssertEqual(loader.loadCommentsCallCount, 1, "Expected no loading request the second time view appears") + } + func test_loadingCommentsIndicator_isVisibleWhileLoadingComments() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() - XCTAssertTrue(sut.isShowingLoadingIndicator, "Expected loading indicator once view is loaded") + sut.simulateAppearance() + XCTAssertTrue(sut.isShowingLoadingIndicator, "Expected loading indicator once view appears") loader.completeCommentsLoading(at: 0) XCTAssertFalse(sut.isShowingLoadingIndicator, "Expected no loading indicator once loading completes successfully") @@ -59,7 +70,7 @@ class CommentsUIIntegrationTests: XCTestCase { let comment1 = makeComment(message: "another message", username: "another username") let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() assertThat(sut, isRendering: [ImageComment]()) loader.completeCommentsLoading(with: [comment0], at: 0) @@ -74,7 +85,7 @@ class CommentsUIIntegrationTests: XCTestCase { let comment = makeComment() let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeCommentsLoading(with: [comment], at: 0) assertThat(sut, isRendering: [comment]) @@ -87,7 +98,7 @@ class CommentsUIIntegrationTests: XCTestCase { let comment = makeComment() let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeCommentsLoading(with: [comment], at: 0) assertThat(sut, isRendering: [comment]) @@ -98,7 +109,7 @@ class CommentsUIIntegrationTests: XCTestCase { func test_loadCommentsCompletion_dispatchesFromBackgroundToMainThread() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() let exp = expectation(description: "Wait for background queue") DispatchQueue.global().async { @@ -111,7 +122,7 @@ class CommentsUIIntegrationTests: XCTestCase { func test_loadCommentsCompletion_rendersErrorMessageOnErrorUntilNextReload() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() XCTAssertEqual(sut.errorMessage, nil) loader.completeCommentsLoadingWithError(at: 0) @@ -124,7 +135,7 @@ class CommentsUIIntegrationTests: XCTestCase { func test_tapOnErrorView_hidesErrorMessage() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() XCTAssertEqual(sut.errorMessage, nil) loader.completeCommentsLoadingWithError(at: 0) @@ -147,7 +158,7 @@ class CommentsUIIntegrationTests: XCTestCase { }).eraseToAnyPublisher() }) - sut?.loadViewIfNeeded() + sut?.simulateAppearance() } XCTAssertEqual(cancelCallCount, 0) diff --git a/EssentialApp/EssentialAppTests/FeedAcceptanceTests.swift b/EssentialApp/EssentialAppTests/FeedAcceptanceTests.swift index cd47a3d2..45495b17 100644 --- a/EssentialApp/EssentialAppTests/FeedAcceptanceTests.swift +++ b/EssentialApp/EssentialAppTests/FeedAcceptanceTests.swift @@ -91,7 +91,9 @@ class FeedAcceptanceTests: XCTestCase { sut.configureWindow() let nav = sut.window?.rootViewController as? UINavigationController - return nav?.topViewController as! ListViewController + let vc = nav?.topViewController as! ListViewController + vc.simulateAppearance() + return vc } private func enterBackground(with store: InMemoryFeedStore) { @@ -106,7 +108,9 @@ class FeedAcceptanceTests: XCTestCase { RunLoop.current.run(until: Date()) let nav = feed.navigationController - return nav?.topViewController as! ListViewController + let vc = nav?.topViewController as! ListViewController + vc.simulateAppearance() + return vc } private func response(for url: URL) -> (Data, HTTPURLResponse) { diff --git a/EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift b/EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift index 18dff504..793ee3c1 100644 --- a/EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift +++ b/EssentialApp/EssentialAppTests/FeedUIIntegrationTests.swift @@ -13,7 +13,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_feedView_hasTitle() { let (sut, _) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() XCTAssertEqual(sut.title, feedTitle) } @@ -24,7 +24,7 @@ class FeedUIIntegrationTests: XCTestCase { var selectedImages = [FeedImage]() let (sut, loader) = makeSUT(selection: { selectedImages.append($0) }) - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [image0, image1], at: 0) sut.simulateTapOnFeedImage(at: 0) @@ -36,10 +36,10 @@ class FeedUIIntegrationTests: XCTestCase { func test_loadFeedActions_requestFeedFromLoader() { let (sut, loader) = makeSUT() - XCTAssertEqual(loader.loadFeedCallCount, 0, "Expected no loading requests before view is loaded") + XCTAssertEqual(loader.loadFeedCallCount, 0, "Expected no loading requests before view appears") - sut.loadViewIfNeeded() - XCTAssertEqual(loader.loadFeedCallCount, 1, "Expected a loading request once view is loaded") + sut.simulateAppearance() + XCTAssertEqual(loader.loadFeedCallCount, 1, "Expected a loading request once view appears") sut.simulateUserInitiatedReload() XCTAssertEqual(loader.loadFeedCallCount, 1, "Expected no request until previous completes") @@ -53,11 +53,22 @@ class FeedUIIntegrationTests: XCTestCase { XCTAssertEqual(loader.loadFeedCallCount, 3, "Expected yet another loading request once user initiates another reload") } + func test_loadFeedActions_runsAutomaticallyOnlyOnFirstAppearance() { + let (sut, loader) = makeSUT() + XCTAssertEqual(loader.loadFeedCallCount, 0, "Expected no loading requests before view appears") + + sut.simulateAppearance() + XCTAssertEqual(loader.loadFeedCallCount, 1, "Expected a loading request once view appears") + + sut.simulateAppearance() + XCTAssertEqual(loader.loadFeedCallCount, 1, "Expected no loading request the second time view appears") + } + func test_loadingFeedIndicator_isVisibleWhileLoadingFeed() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() - XCTAssertTrue(sut.isShowingLoadingIndicator, "Expected loading indicator once view is loaded") + sut.simulateAppearance() + XCTAssertTrue(sut.isShowingLoadingIndicator, "Expected loading indicator once view appears") loader.completeFeedLoading(at: 0) XCTAssertFalse(sut.isShowingLoadingIndicator, "Expected no loading indicator once loading completes successfully") @@ -76,7 +87,7 @@ class FeedUIIntegrationTests: XCTestCase { let image3 = makeImage(description: nil, location: nil) let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() assertThat(sut, isRendering: []) loader.completeFeedLoading(with: [image0, image1], at: 0) @@ -96,7 +107,7 @@ class FeedUIIntegrationTests: XCTestCase { let image1 = makeImage() let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [image0], at: 0) assertThat(sut, isRendering: [image0]) @@ -113,7 +124,7 @@ class FeedUIIntegrationTests: XCTestCase { let image0 = makeImage() let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [image0], at: 0) assertThat(sut, isRendering: [image0]) @@ -128,7 +139,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_loadFeedCompletion_dispatchesFromBackgroundToMainThread() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() let exp = expectation(description: "Wait for background queue") DispatchQueue.global().async { @@ -141,7 +152,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_loadFeedCompletion_rendersErrorMessageOnErrorUntilNextReload() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() XCTAssertEqual(sut.errorMessage, nil) loader.completeFeedLoadingWithError(at: 0) @@ -154,7 +165,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_tapOnErrorView_hidesErrorMessage() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() XCTAssertEqual(sut.errorMessage, nil) loader.completeFeedLoadingWithError(at: 0) @@ -168,7 +179,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_loadMoreActions_requestMoreFromLoader() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading() XCTAssertEqual(loader.loadMoreCallCount, 0, "Expected no requests before until load more action") @@ -195,8 +206,8 @@ class FeedUIIntegrationTests: XCTestCase { func test_loadingMoreIndicator_isVisibleWhileLoadingMore() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() - XCTAssertFalse(sut.isShowingLoadMoreFeedIndicator, "Expected no loading indicator once view is loaded") + sut.simulateAppearance() + XCTAssertFalse(sut.isShowingLoadMoreFeedIndicator, "Expected no loading indicator once view appears") loader.completeFeedLoading(at: 0) XCTAssertFalse(sut.isShowingLoadMoreFeedIndicator, "Expected no loading indicator once loading completes successfully") @@ -216,7 +227,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_loadMoreCompletion_dispatchesFromBackgroundToMainThread() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(at: 0) sut.simulateLoadMoreFeedAction() @@ -230,7 +241,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_loadMoreCompletion_rendersErrorMessageOnError() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading() sut.simulateLoadMoreFeedAction() @@ -245,7 +256,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_tapOnLoadMoreErrorView_loadsMore() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading() sut.simulateLoadMoreFeedAction() @@ -266,7 +277,7 @@ class FeedUIIntegrationTests: XCTestCase { let image1 = makeImage(url: URL(string: "http://url-1.com")!) let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [image0, image1]) XCTAssertEqual(loader.loadedImageURLs, [], "Expected no image URL requests until views become visible") @@ -283,7 +294,7 @@ class FeedUIIntegrationTests: XCTestCase { let image1 = makeImage(url: URL(string: "http://url-1.com")!) let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [image0, image1]) XCTAssertEqual(loader.cancelledImageURLs, [], "Expected no cancelled image URL requests until image is not visible") @@ -299,7 +310,7 @@ class FeedUIIntegrationTests: XCTestCase { let image1 = makeImage(url: URL(string: "http://url-1.com")!) let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [image0, image1]) sut.simulateFeedImageBecomingVisibleAgain(at: 0) @@ -314,7 +325,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_feedImageViewLoadingIndicator_isVisibleWhileLoadingImage() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [makeImage(), makeImage()]) let view0 = sut.simulateFeedImageViewVisible(at: 0) @@ -338,7 +349,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_feedImageView_rendersImageLoadedFromURL() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [makeImage(), makeImage()]) let view0 = sut.simulateFeedImageViewVisible(at: 0) @@ -360,7 +371,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_feedImageViewRetryButton_isVisibleOnImageURLLoadError() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [makeImage(), makeImage()]) let view0 = sut.simulateFeedImageViewVisible(at: 0) @@ -385,7 +396,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_feedImageViewRetryButton_isVisibleOnInvalidImageData() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [makeImage()]) let view = sut.simulateFeedImageViewVisible(at: 0) @@ -401,7 +412,7 @@ class FeedUIIntegrationTests: XCTestCase { let image1 = makeImage(url: URL(string: "http://url-1.com")!) let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [image0, image1]) let view0 = sut.simulateFeedImageViewVisible(at: 0) @@ -424,7 +435,7 @@ class FeedUIIntegrationTests: XCTestCase { let image1 = makeImage(url: URL(string: "http://url-1.com")!) let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [image0, image1]) XCTAssertEqual(loader.loadedImageURLs, [], "Expected no image URL requests until image is near visible") @@ -440,7 +451,7 @@ class FeedUIIntegrationTests: XCTestCase { let image1 = makeImage(url: URL(string: "http://url-1.com")!) let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [image0, image1]) XCTAssertEqual(loader.cancelledImageURLs, [], "Expected no cancelled image URL requests until image is not near visible") @@ -454,7 +465,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_feedImageView_configuresViewCorrectlyWhenTransitioningFromNearVisibleToVisibleWhileStillPreloadingImage() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [makeImage()]) sut.simulateFeedImageViewNearVisible(at: 0) @@ -475,7 +486,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_feedImageView_configuresViewCorrectlyWhenCellBecomingVisibleAgain() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [makeImage()]) let view0 = sut.simulateFeedImageBecomingVisibleAgain(at: 0) @@ -495,7 +506,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_feedImageView_doesNotShowDataFromPreviousRequestWhenCellIsReused() throws { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [makeImage(), makeImage()]) let view0 = try XCTUnwrap(sut.simulateFeedImageViewVisible(at: 0)) @@ -509,7 +520,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_feedImageView_doesNotRenderLoadedImageWhenNotVisibleAnymore() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [makeImage()]) let view = sut.simulateFeedImageViewNotVisible(at: 0) @@ -521,7 +532,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_loadImageDataCompletion_dispatchesFromBackgroundToMainThread() { let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [makeImage()]) _ = sut.simulateFeedImageViewVisible(at: 0) @@ -536,7 +547,7 @@ class FeedUIIntegrationTests: XCTestCase { func test_feedImageView_doesNotLoadImageAgainUntilPreviousRequestCompletes() { let image = makeImage(url: URL(string: "http://url-0.com")!) let (sut, loader) = makeSUT() - sut.loadViewIfNeeded() + sut.simulateAppearance() loader.completeFeedLoading(with: [image]) sut.simulateFeedImageViewNearVisible(at: 0) diff --git a/EssentialApp/EssentialAppTests/Helpers/ListViewController+TestHelpers.swift b/EssentialApp/EssentialAppTests/Helpers/ListViewController+TestHelpers.swift index 391c3da5..726b37c4 100644 --- a/EssentialApp/EssentialAppTests/Helpers/ListViewController+TestHelpers.swift +++ b/EssentialApp/EssentialAppTests/Helpers/ListViewController+TestHelpers.swift @@ -6,10 +6,49 @@ import UIKit import EssentialFeediOS extension ListViewController { - public override func loadViewIfNeeded() { - super.loadViewIfNeeded() + func simulateAppearance() { + if !isViewLoaded { + loadViewIfNeeded() + prepareForFirstAppearance() + } - tableView.frame = CGRect(x: 0, y: 0, width: 1, height: 1) + beginAppearanceTransition(true, animated: false) + endAppearanceTransition() + } + + private func prepareForFirstAppearance() { + setSmallFrameToPreventRenderingCells() + replaceRefreshControlWithSpyForiOS17Support() + } + + private func setSmallFrameToPreventRenderingCells() { + tableView.frame = CGRect(x: 0, y: 0, width: 390, height: 1) + } + + private func replaceRefreshControlWithSpyForiOS17Support() { + let spyRefreshControl = UIRefreshControlSpy() + + refreshControl?.allTargets.forEach { target in + refreshControl?.actions(forTarget: target, forControlEvent: .valueChanged)?.forEach { action in + spyRefreshControl.addTarget(target, action: Selector(action), for: .valueChanged) + } + } + + refreshControl = spyRefreshControl + } + + private class UIRefreshControlSpy: UIRefreshControl { + private var _isRefreshing = false + + override var isRefreshing: Bool { _isRefreshing } + + override func beginRefreshing() { + _isRefreshing = true + } + + override func endRefreshing() { + _isRefreshing = false + } } func simulateUserInitiatedReload() { diff --git a/EssentialFeed/EssentialFeediOS/Shared UI/Controllers/ListViewController.swift b/EssentialFeed/EssentialFeediOS/Shared UI/Controllers/ListViewController.swift index 0964720c..bc54e774 100644 --- a/EssentialFeed/EssentialFeediOS/Shared UI/Controllers/ListViewController.swift +++ b/EssentialFeed/EssentialFeediOS/Shared UI/Controllers/ListViewController.swift @@ -14,6 +14,8 @@ public final class ListViewController: UITableViewController, UITableViewDataSou } }() + private var onViewIsAppearing: ((ListViewController) -> Void)? + public var onRefresh: (() -> Void)? public override func viewDidLoad() { @@ -21,9 +23,19 @@ public final class ListViewController: UITableViewController, UITableViewDataSou configureTableView() configureTraitCollectionObservers() - refresh() + + onViewIsAppearing = { vc in + vc.onViewIsAppearing = nil + vc.refresh() + } } + public override func viewIsAppearing(_ animated: Bool) { + super.viewIsAppearing(animated) + + onViewIsAppearing?(self) + } + private func configureTableView() { dataSource.defaultRowAnimation = .fade tableView.dataSource = dataSource