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

Feature Request: Allow tests to specify environment #167

Open
Tyler-Keith-Thompson opened this issue Mar 14, 2022 · 4 comments
Open

Feature Request: Allow tests to specify environment #167

Tyler-Keith-Thompson opened this issue Mar 14, 2022 · 4 comments
Labels
feature request New feature or request

Comments

@Tyler-Keith-Thompson
Copy link
Contributor

Tyler-Keith-Thompson commented Mar 14, 2022

The problem

I've been running into a plethora of EnvironmentObject issues. Seemingly, nested views that rely on it have lots of issues finding it after state changes.

My specific use-case could be summed up as, I have a view heavily dependent on an environment object, I want ViewInspector to prove that the parent displays that view as a sheet after a state change. Unfortunately, before the state change in the parent, it has the environment objects, but after it keeps telling me the environment objects aren't there. I've also had BAD_ACCESS instructions and other bizarre behavior when ViewInspector tries to handle the environment itself.

While this specific problem could be tracked down this smacks to me of ViewInspector constantly having to track down and attempt to duplicate very complex environment scenarios.

Potential Solution

Let consumers of the library specify the environment to ViewInspector. I believe this has several advantages:

  • While consumers can use ViewHosting.view(someView.environmentObject(injected)) this allows more explicit control with arrange-act-assert based tests. (Given my environment is , When my view , Then )
  • It's possible, although I'd need to investigate further, that this would negate the need for the custom Inspection class to be injected into prod code. I think this is true because I believe nested views would largely be functional, therefore you could create a view wrapper that did much the same thing as Inspection
  • While this would put the library more into "I'm going to mimic what SwiftUI does" territory, the current ViewHosting solution demonstrably does not behave as it does in production anyways, so rather than tracking down a litany of problems, this might allow us to give more stability to tests using ViewInspector. If Apple exposes a better way, or we discover one, of hosting views and correctly setting up the environment we can change implementation with little consumer impact.

I'll stop there because it's a lot to read. If there's interest, I'm happy to create a discussion and talk through some of my API design ideas. I am also willing to do the grunt work and submit a PR, assuming others believe this to be a worthwhile approach.

@nalexn
Copy link
Owner

nalexn commented Mar 15, 2022

Hey @Tyler-Keith-Thompson , the EnvironmentObject injection is a real painpoint, and obviously we need to fix the library crashing in the described use case.
I'm not strongly against opening the API for explicit injection of the Environment into the unwrapped children; this code is currently private in the library (see EnvironmentInjection.swift).

The way it works is that after we extracted the child as a struct, we patch its internal references to point to the EnvironmentObject inherited from the parent before returning to the user.

Apparently, after the state changes for the child, a new struct is created and it needs to be patched again.

@Tyler-Keith-Thompson
Copy link
Contributor Author

Cool, I'll work on a PR. FWIW my use-case seems to be more complex because distilling it down to a minimally reproducible example has been quite challenging.

Regardless, I think this feature will help.

@nh7a
Copy link
Contributor

nh7a commented May 8, 2022

I'm using ViewHosting.view(myView.environmentObject(appState)) and "async inspection", and it appears to have solved my problem on my laptop but it still 100% crashes on CI (GitHub Action & Xcode Cloud). Hopefully your idea fully solves the problem.

@huwshimi
Copy link

Cool, I'll work on a PR. FWIW my use-case seems to be more complex because distilling it down to a minimally reproducible example has been quite challenging.

I believe this is a minimal reproduction of the problem.

A view is displayed on a state change which requires access to the environment context. This works fine when running the app, but fails in the test with XCTAssertNotNil failed: throwing "+entityForName: nil is not a legal NSPersistentStoreCoordinator for searching for entity name 'Item'".

Hope that helps track down the issue.

@nalexn nalexn added the feature request New feature or request label Sep 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants