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

StaticContext container binding does not properly override bindings in SceneTests #88

Open
chrisschwartzdev opened this issue Feb 8, 2020 · 10 comments

Comments

@chrisschwartzdev
Copy link

chrisschwartzdev commented Feb 8, 2020

Describe the bug
Following this example, I should be able to override a binding in SceneContext or ProjectContext using StaticContext in a SceneTestFixture.

However, this doesn't seem to be the case, unless I am misunderstanding the example.

To Reproduce
Steps to reproduce the behavior:

  1. Make a new Unity project with Exten/Zenject and the test framework added.
  2. Create a folder named "Tests" and an asmdef for a test assembly, referencing Zenject, Zenject-TestFramework, and UnityEngine.TestRunner.
  3. Create a scene test in the folder with Right Click > Zenject > Scene Test.
  4. Create a MonoInstaller to be used in the ProjectContext, create the ProjectContext in resources folder, and add a SceneContext to the "SampleScene" scene.
  5. Override InstallBindings in the ProjectInstaller class and bind a new, simple class (i.e. a Cat object with a Name property), give the Cat instance the name "Whiskers".
  6. Modify the sample test in the scene test to override the binding of "Cat" using a new instance with the name "Ajax".
  7. Load the "SampleScene" scene with yield return LoadScene("SampleScene") and get the resolved Cat instance with SceneContainer.Resolve<Cat>() after loading.
  8. Add an assert equal statement expecting the name "Ajax" and pass the actual Cat.Name).
  9. Run the test from the PlayMode tab of the Test Runner.

Expected behavior
If I'm understanding the above example correctly, the StaticContext binding should be overriding the ProjectContext binding, and the assertion should pass. The expected name of the resolved Cat instance from the SceneContainer should be "Ajax", but it is actually "Whiskers".

Here is the entirety of my ProjectInstaller class, with the Cat class:

    public class ProjectInstaller : MonoInstaller
    {
        public override void InstallBindings()
        {
            Container.BindInstance(new Cat {Name = "Whiskers"});
        }
    }

    public class Cat
    {
        public string Name { get; set; }
    }

and here is the entirety of my test class:

    public class UntitledSceneTest : SceneTestFixture
    {
        [UnityTest]
        public IEnumerator TestScene()
        {
            StaticContext.Container.BindInstance(new Cat {Name = "Ajax"}).AsSingle();
            
            yield return LoadScene("SampleScene");

            var cat = SceneContainer.Resolve<Cat>();
            Assert.AreEqual("Ajax", cat.Name);
        }
    }

Extenject and Unity info (please complete the following information):

  • Zenject version: 9.1.0
  • Unity version: Unity 2019.3.0f6
  • Project's scripting backend [e.g. Mono/IL2CPP]: Mono

Additional context
After some debugging, I suspect that the order of binding is going ProjectContext > StaticContext > ProjectContext, as if ProjectContext is calling InstallBindings once more at the end, and overriding the overridden binding. I tried moving the StaticContext binding to after the scene loads, as well as trying a bunch of different attachments to the binding call (i.e. AsSingle, AsTransient, AsCached) with no luck at all.

@chrisschwartzdev
Copy link
Author

chrisschwartzdev commented Feb 8, 2020

I spent some more time tonight trying to copy the example test into my new project to make sure I wasn't missing something obvious, and I'm still getting the same unexpected result.

If I try to override the EnemySpawner.Settings binding in the test to set NumEnemiesStartAmount to 1, but bind an instance with NumEnemiesStartAmount to 5 in ProjectContext (or SceneContext), it will resolve an EnemyRegistry instance with a dummy list of enemies with 5 items.

Again, this is assuming that in the hypothetical example, EnemySpawner.Settings is being bound somewhere else (either with ZenjectBinding, or an installer on SceneContext or ProjectContext), which the example suggests, to me.

@Haelle
Copy link

Haelle commented Feb 12, 2020

Same problem here.

A binding on the ProjectContext is not overriden by the StaticContext (there is a SceneContext on the scene)

Extenject version : 9.1.0
Unity version : 2019.2.20.f1

FYI :

StaticContext.Bind<Foo>.AsSingle();
yield return LoadScene("MyScene");

StaticContext.Resolve<Foo>(); // instance A
SceneContainer.Resolve<Foo>(); // instance B
ProjectContext.Instance.Container.Resolve<Foo>(); // instance B
// last but not least : 
ProjectContext.Instance.Container.ParentContainers.First().Resolve<Foo>(); // instance A !!!

@Haelle
Copy link

Haelle commented Feb 12, 2020

StaticContext is used in last resort if no other "closer" context contains the expected binding. And indeed the documentation does not say that it will override other bindings (StaticContext is the parent context of ProjectContext, and so will be inherited by all dependencies), so the behavior is as written in the documentation, but not as @BassaForte and I expected.

Details

So I've made some investigations, in my tests I do something like SceneContainer.Resolve<Foo>(); and I ended up here :

https://github.com/svermeulen/Extenject/blob/a8a104383e0558690571eeb696acba2e263c5007/UnityProject/Assets/Plugins/Zenject/Source/Main/DiContainer.cs#L556

Calling GetContainerHeirarchyDistance on all Container found. But the distance increase each time we go further/deeper of the "starting" container. And StaticContext.Container is the farthest as it is on top of all others, it will never be chosen except if it is the only one with the expected binding.

P.S: let us pray the debugger :)

@chrisschwartzdev
Copy link
Author

@Haelle Interesting. Thanks for investigating and thanks for the details. I ended up using a crappy workaround for my tests by resolving the current instance of my binding by using Unity's Object.FindObjectOfType<SceneContext> at the beginning of the test, then modifying properties for my test.

Not ideal, but I guess it'll have to do until there's a better solution or the documentation is updated.

@Haelle
Copy link

Haelle commented Feb 12, 2020

How can you find you object before calling yield return LoadScene('bar') ? When this return everything is binded and it's too late :/

I ended up resolving my object and modifying it just after the LoadScene, I can do it because I'm just mocking a Property, if it was a method it wouldn't work (and maybe I shouldn't do it also)

@chrisschwartzdev
Copy link
Author

You can't. Sorry if it wasn't clear but I'm doing the same thing you mentioned - resolving the object after LoadScene and modifying properties. That's the main limitation of the workaround, aside from it overall not being a great solution.

@denravonska
Copy link

The scene tests documentation has an example where it overrides the enemy spawner via StaticContext before loading the scene. This doesn't work, as you mention, so the doc is either incorrect or I'm missing something else.

@chuckingbricks
Copy link

chuckingbricks commented Sep 1, 2022

I'm running into the same issue where I try to override a binding through the StaticContext before calling LoadScene with a similar example as the one in the docs. So it possible that it isnt working as documented.

@denravonska
Copy link

My workaround for this was to add a static callback to my installer which the tests can hook into and override whatever needs to be mocked.

public class ComponentInstaller : MonoInstaller<ComponentInstaller>
{
   public static Action<DiContainer> LateInstaller;

   public override void InstallBindings()
   {
      // Program's regular bindings go here
      Container.BindInterfacesTo....
      
      // Let tests override and rebind
      LateInstaller?.Invoke(Container);
   }
}

Not the prettiest but it works.

@chuckingbricks
Copy link

chuckingbricks commented Mar 13, 2023

Following up on this topic. The sample SceneTestFixture test, where a binding on StaticContext.Container is used to override a binding in the scene does work. The in the SpaceShip sample scene, the installer binds uses .IfNotBound() to bind the instance.

Since the scene test is binding the instance through the StaticContext before the scene is loaded, the binding in the installer is skipped/overridden.

.https://github.com/Mathijs-Bakker/Extenject/blob/d92b8cbe8b3563dc55b3c92072dde54792fef0df/UnityProject/Assets/Plugins/Zenject/Samples/SampleGame2%20(Advanced)/Scripts/Installers/GameSettingsInstaller.cs#L52

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

No branches or pull requests

4 participants