Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broken tests with Xcode 16-RC / iOS 18 #1303

Closed
jdev7 opened this issue Sep 10, 2024 · 17 comments · Fixed by #1307
Closed

Broken tests with Xcode 16-RC / iOS 18 #1303

jdev7 opened this issue Sep 10, 2024 · 17 comments · Fixed by #1307

Comments

@jdev7
Copy link

jdev7 commented Sep 10, 2024

We are experiencing failures in tests written with KIF, and we aren't sure if it's related to the framework and the new versions of Xcode & iOS, or it is a more general problem.

In our project we have both UITests running with XCUITest and KIF. However the first ones are working as they were with Xcode 15.4, but not so much when running KIF-based ones.

The most common error we're finding is the following:

KIFTestActor.m:89: Expected dequeued view to be returned to the collection view in preparation for display. When the collection view's data source is asked to provide a view for a given index path, ensure that a single view is dequeued and returned to the collection view. Avoid dequeuing views without a request from the collection view. For retrieving an existing view in the collection view, use -[UICollectionView cellForItemAtIndexPath:] or -[UICollectionView supplementaryViewForElementKind:atIndexPath:]. Dequeued view: <XXXModule.XXXValuePropositionCell: 0x15fa73180; baseClass = UICollectionViewCell; frame = (383 0; 375 415); layer = <CALayer: 0x600004397bc0>>; Collection view: <UICollectionView: 0x1211e8200; frame = (0 0; 375 415); clipsToBounds = YES; gestureRecognizers = <NSArray: 0x6000011effc0>; backgroundColor = UIExtendedGrayColorSpace 0 0; layer = <CALayer: 0x6000044eb020>; contentOffset: {0, 0}; contentSize: {375, 415}; adjustedContentInset: {0, 0, 0, 0}; layout: <UICollectionViewCompositionalLayout: 0x1225a70f0>; dataSource: <XXXModule.XXXValuePropositionsCarouselView: 0x12259df10; frame = (0 140; 375 423); layer = <CALayer: 0x6000044eb120>>> (NSInternalInconsistencyException)

From our initial investigations, we have some screens in which we have a basic implementation, returning just one cell type, which is registered, while in some others we have a guard clause returning UICollectionViewTableViewCell() in the else clause, in which we have placed a breakpoint and wasn't hit while running tests and failing.

Furthermore, not running the app in a device, nor running XCUITest based tests we experience similar failures. Similarly, if I run KIF tests targeting iOS 17.x in the same Xcode 16-RC they don't fail, so the problem seems to be related to iOS 18.

Any help or guidance would be much appreciated.

@ispatel22
Copy link

We ran into a similar issue with KIF. It manually calls the cellForItemAtIndexPath method, which leads to errors about dequeuing cells multiple times. This has always been a problem, but with Xcode 16, it throws an Assert for multiple dequeues, whereas Xcode 15 just logged a warning. We haven't found a permanent fix yet, but we do have a workaround. The workaround is to do the following in cellForItemAtIndexPath for all the collection views that are crashing. Do this check as the first thing in the method.

#if TESTING
    NSArray<NSString *> *callStackSymbols = [NSThread callStackSymbols];
    if ([callStackSymbols[1] containsString:@"KIF"])
    {
        return [collectionView cellForItemAtIndexPath:indexPath];
    }
#endif

If this doesn't work there were couple other threads on Apple forums that suggest a different fix.
Somebody did try to fix this issue but it got reverted due to other failure: #1252

@danielbarela
Copy link
Contributor

This is also happening when using SwiftUI lists that are long enough to dequeue new cells when scrolling to a label.

@justinseanmartin
Copy link
Contributor

I think probably the best fix here is to implement the same logic that we have for UITableViews for UICollectionViews. Instead of enumerating and popping elements programmatically, instead scroll the view so that the elements is on screen, then use the cell on the screen. I'll take a crack at this today, as we have a need for this for other reasons (setting properties and child views on willAppear instead of on cell init).

@jdev7
Copy link
Author

jdev7 commented Sep 16, 2024

Hi @justinseanmartin! Were you able to test your solution? Any insights? Thanks!

@justinseanmartin
Copy link
Contributor

Hey, sorry, had a team offsite last week and didn't get to dig in. I'm working on #1300 today. Once that's green again, I'll start working on the fix for this.

@justinseanmartin
Copy link
Contributor

Fix was posted in #1307. Please take a look and give that a test locally to make sure it doesn't cause other issue. Thanks!

@cerihughes
Copy link

Hey! Thanks for looking into this!

I've tried that version out with out KIF tests and I'm seeing a lot of UICollectionViewCell returned from cellForItemAtIndexPath is unexpectedly nil! (NSInternalInconsistencyException) failures.

The majority of our tests are passing but unfortunately I'm not able to figure out the use case for the failure yet.

I see this on both iOS17.5 and iOS18. The previous version (3.9.0) wasn't causing issues on iOS17.5.

@cerihughes
Copy link

I also tried out another variant where I replaced

CGRect visibleRect = [collectionView layoutAttributesForItemAtIndexPath:indexPath].frame;
[collectionView scrollRectToVisible:visibleRect animated:NO];

with

[collectionView scrollToItemAtIndexPath:indexPath atScrollPosition:UICollectionViewScrollPositionNone animated:NO];

Weirdly, I see that some tests that failed with the 1st variant now pass with the 2nd, but it doesn't seem to fix every case.

@cerihughes
Copy link

I spent some time this morning trying to reproduce our more complex UICollectionView-based UI in the Test Host app but I couldn't get it to fail. I'm still looking for more clues about why ours is failing, but it would be interesting to see if anyone else is still seeing problems. Thanks again!

@cerihughes
Copy link

I've tried that version out with out KIF tests and I'm seeing a lot of UICollectionViewCell returned from cellForItemAtIndexPath is unexpectedly nil! (NSInternalInconsistencyException) failures.

One other thing to note: I tried removing this NSAssert line in the hope this somehow this call gets retried and eventually finds the cell... but I still see failures on both iOS17.5 and iOS18.

@justinseanmartin
Copy link
Contributor

I'm actually hitting the same UICollectionViewCell returned from cellForItemAtIndexPath is unexpectedly nil in our full suite. I think it is happening in cases where there are collection views nested within collection views. I've got a proposed fix that I'm working on, and I'll put up a branch that you can give a shot and see if it helps in your case as well. I've still got more issues I'm working through before I cut a release with all the changes I've landed in the last week, but I'm actively working through that.

@cerihughes
Copy link

That's great. Thanks so much. Yes - our cases are regularly with collection views within other collections views so I suspect the issue you're working on will fix it for us. I'll keep an eye out for the fix and test as soon as it's ready. Thanks again!

@justinseanmartin
Copy link
Contributor

Could you try out the fix that is a WIP here: https://github.com/justinseanmartin/KIF/tree/jmartin/collection-view-cell-spin-runloop and see if that fully addresses it for you?

@cerihughes
Copy link

I've tried that branch out. Initially, I was seeing that it fixed a lot of the issues, but then I noticed that a lot of the tests aren't passing consistently - if I run a test repeatedly in Xcode, some of the tests have a ~65% failure rate for example.

Is there some debug information I can capture that will help show the difference between the passing and failing cases?

@justinseanmartin
Copy link
Contributor

Do you have any tests that are consistently failing or is it only flakiness now? We could try increasing it to 100 iterations of the runloop being spun instead of 10. Maybe we should make it explicitly time based, like spin for up to 1s (maybe a configurable value). It is probably better to give more time to avoid tests having flakiness from race conditions, but it also could lead to tests running really slow if they're spending a lot of time scrolling through collection views (especially ones with lots of elements in them).

If you wanted to experiment in your own branch to see what seems to work for you, that'd help. I'm seeing some tests still failing on this assertion locally, so I'm still iterating on that fix myself as well.

@cerihughes
Copy link

I am seeing some tests that are constently failing, but these are more related to accessibility identifiers not being found during the run of a block predicate... I'm still investigating those, but it's likely not related as there's no collection view in the UI.

We've seen a few failures that are related to changes in UIKit under the hood so it may be another case of that.

There is another consistent failure where a label in a collection view cell is being updated but the ui changes never get reflected in the UI. That's a strange one but I'll look into that some more and get back to you.

In the meantime, I'll have a play around with the number of iterations in that while loop and see if it changes anything.

Thanks again for the support! 🙌

@cerihughes
Copy link

I've spent a couple of days testing the version at https://github.com/justinseanmartin/KIF/tree/jmartin/collection-view-cell-spin-runloop with an iteration count of 100 and things definitely seem more stable. This has allowed me to dig into the actually failures we were seeing and update the tests accordingly.

At this stage, we're running tests using iOS17.5 against this new version of KIF to make sure we have no regressions. We'll address the iOS18 issues (and anything that brings up) once we're happy with the iOS17.5 runs.

There's one category of failure I'm seeing and after digging into this a little, I realise that things will run slightly differently with the changes made recently to KIF. Technically, some tests that would pass in the 3.9.0 release of KIF will now fail so this may be worth communicating when the release goes out. To be fair, this may be down to our tests not waiting for the right conditions (and passing by coincidence previously), and the workaround I have seems very sensible.

Some tests are triggering a scroll operation in a collection view and waiting for a UILabel to appear on-screen. Once that label is on-screen, the test will then look for a UIPageControl (which is outside the collection view) and check that its state has updated (which is itself dependent on the scroll position of the collection view).

In some tests, the label will appear on-screen before the UIPageControl is updated (i.e. the label is visible but we haven't scrolled enough to trigger a UIPageControl change), so the search for the page control will start while the collection view is still scrolling.

In the old KIF, this was fine as the collection view would continue to scroll to where it was originally heading and eventually the page control would update and the test would pass.
In the new KIF, the search for the page control will "interrupt" the collection view's scroll so it will never get to a point where the page control updates and the test would fail.

A simple waitForAnimationsToFinish() call after the scroll operation fixes this as it ensures that the collection view has stopped scrolling and the UIPageControl has updated before we start searching through the view hierarchy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants