ViewInspector icon indicating copy to clipboard operation
ViewInspector copied to clipboard

ViewInspector inspects "destination" property of NavigationLink, can recurse infinitely

Open varun531 opened this issue 2 years ago • 5 comments

Using ViewInspector 0.9.1, XCode 13.2.1.

If I have the following views:


struct ViewA: View {
    var body: some View {
        Text("viewA")
        NavigationLink("Goto view B", destination: ViewB())
    }
}

struct ViewB: View {
    var body: some View {
        Text("viewB")
    }
}

and I try to search for the text "viewB" on ViewA via:

let sut = ViewA()
XCTAssertNotNil(try sut.inspect().find(text: "viewB"))

This test will actually pass for me, because ViewInspector treats the NavigationLink as a child of ViewA, and its destination as a child of the NavigationLink.

NOTE: If ViewB has a NavigationLink back to ViewA, ViewInspector will recurse infinitely while inspecting ViewA.

varun531 avatar Mar 14 '22 09:03 varun531

To try and be a good community member I thought I'd throw in my $0.02. @varun531 what is your expected ViewInspector behavior?

It occurs to me you could try to bound find so that it'll only look in destinations with active navigation links. It also occurs to me you effectively created an infinite view hierarchy, so it makes a lot of sense why ViewInspector recurses infinitely.

An alternative approach would be to have find internally have some configurable recursion depth to avoid infinitely doing it, but I find that sub-optimal because it no longer allows arbitrarily nested views.

Tyler-Keith-Thompson avatar Mar 14 '22 15:03 Tyler-Keith-Thompson

@Tyler-Keith-Thompson it should NOT be inspecting the destination property of the NavigationLink when inspecting ViewA. Inspection/verification of ViewA content is meaningless if it ends up inspecting ALL views that can be reached from ViewA via NavigationLinks.

varun531 avatar Mar 14 '22 17:03 varun531

@Tyler-Keith-Thompson I agree that having a configurable recursion depth is bad, because it limits arbitrarily nested views. I would be happy having find skip over destinations of inactive NavigationLinks. Regardless, I hope you agree that the scenario I posted above is unacceptable, because it seriously screws up the signal of the tests.

varun531 avatar Mar 15 '22 18:03 varun531

@nalexn Any thoughts on this? Consider the following scenario to understand why this bug is so pressing:

Let's say I have the following views connected by NavigationLinks:

A -> B -> C -> D -> E -> F -> G -> H

Inspecting A's contents will end up inspecting H's contents, so if the content you're looking for on A is not on A, but on H, inspection of A will pass, which is surely not what you want.

varun531 avatar Mar 31 '22 00:03 varun531

Yeah, I see. I suspect the correct find behavior should be to continue the search in the NavigationLinks's destination only if it is active. I'll look into a fix

nalexn avatar Mar 31 '22 08:03 nalexn

I've added a fix to both infinite recursion and the destination view being accessible for inactive links. FYI, for the latter, the check is only employed for NavigationLinks with Binding parameter for "selection". Without an explicit selection parameter, it is currently impossible to reliably identify active / inactive links, so it falls back to the old behavior, that is to access the destination regardless of the link selection state.

nalexn avatar Jan 08 '23 13:01 nalexn

Fixed released with v0.9.4

nalexn avatar Jan 14 '23 18:01 nalexn