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

ObjectContainer.IsRegistered<T>() does not walk up the inheritance chain, even though Resolve<T>() does, giving misleading results #2759

Open
DrEsteban opened this issue Aug 9, 2024 · 9 comments
Labels

Comments

@DrEsteban
Copy link

DrEsteban commented Aug 9, 2024

SpecFlow Version

3.9.74

Which test runner are you using?

xUnit

Test Runner Version Number

2.4.1

.NET Implementation

.NET 8.0

Project Format of the SpecFlow project

Sdk-style project format

.feature.cs files are generated using

SpecFlow.Tools.MsBuild.Generation NuGet package

Test Execution Method

Visual Studio Test Explorer

SpecFlow Section in app.config or content of specflow.json

{
"$schema": "https://specflow.org/specflow-config.json",
"stepAssemblies": [
{
"assembly": "<my_common_assembly>"
}
]
}

Issue Description

I am registering services in the Global container in a [BeforeTestRun] method due to Issue #2580.

[BeforeTestRun(Order = 10)]
public static void SetEnvironmentConfiguration(ObjectContainer container)
{
    // Registering to the base container is necessary due to an integration issue with XUnit:
    // https://github.com/SpecFlowOSS/SpecFlow/issues/2580
    container.BaseContainer.RegisterInstanceAs<IEnvironmentConstantsProvider>(EnvironmentConstants.Instance);
}

In another portion of my project, I am attempting to proactively check whether this dependency has been registered so I can give a helpful error message:

if (!container.IsRegistered<IEnvironmentConstantsProvider>())
{
    throw new Exception($"{nameof(IEnvironmentConstantsProvider)} not registered in container. See <docs_on_how_to_configure_correctly>");
}

var envProvider = container.Resolve<IEnvironmentConstantsProvider>();

This call to IsRegistered() is returning false, even though the object is registered, just not at the hierarchy level of the current ObjectContainer... If I comment out my if check, the object resolves as expected because Resolve() actually walks up the inheritance chain to find an object. But it seems as if IsRegistered() simply stops and returns false if it isn't in the immediate scope of the current ObjectContainer - which is not what I'd expect as a user.

NOTE: I've devised a workaround by introducing an extension method as follows, which works:

public static bool IsRegisteredAtAnyLevel<T>(this IObjectContainer container)
{
    do
    {
        if (container.IsRegistered<T>())
        {
            return true;
        }
    } while (container is ObjectContainer c && (container = c.BaseContainer) != null);

    return false;
}

Steps to Reproduce

  1. Register an object at the TestThread or Feature level.
  2. Call container.IsRegistered<MyObj>() at the scenario level => FAIL
  3. Call container.Resolve<MyObj>() at the scenario level => SUCCESS

My issue description also provides a decent description of the repro.

Link to Repro Project

Closed source project

@DrEsteban DrEsteban added the Bug label Aug 9, 2024
@DrEsteban
Copy link
Author

FYI - I'd also recommend updating your issue template to include .NET 8.0. The max it allows right now is .NET 6.0

@DrEsteban
Copy link
Author

DrEsteban commented Aug 9, 2024

Existing issue on BoDi: SpecFlowOSS/BoDi#18

@gkalnytskyi-woolworthslimited

@DrEsteban, I would suggest checking Reqnroll for a compatible alternative and check if this bug is reproducible there, as this project hasn't been updated in more than a year.

@DrEsteban
Copy link
Author

😯 I didn't realize! Thanks for the recommendation @gkalnytskyi-woolworthslimited

@DeLaphante
Copy link

DeLaphante commented Nov 6, 2024

I would advised all to not use Bodi - but use ScenarioContainer instead thats built into Specflow

Specflow is working perfectly fine for me without Bodi in the following daily github actions repo -> https://github.com/DeLaphante/CynkyAutomation/actions

@DrEsteban
Copy link
Author

@DeLaphante I'm not sure your comment is applicable to this issue? 🤔 It doesn't really have anything to do with the test runner you use or anything to do with LivingDoc.

@DeLaphante
Copy link

DeLaphante commented Nov 11, 2024

@DrEsteban -Sorry i didn't review my comment 🤦‍♂️- I've provided a link to my repo so you can see how Specflow is working perfectly fine when everything is up to date and you use ScenarioContainer - if you don't want to do the migration to Reqnroll then you can continue to use Specflow without any issue by removing Bodi

@DrEsteban
Copy link
Author

DrEsteban commented Nov 11, 2024

@DeLaphante thanks for updating your comment! However I'm still confused...

SpecFlow inherently uses Bodi. It's hard coded into the project. In fact, I believe Bodi was specifically created for use in SpecFlow to support its scenarios 😄 You have to actually do work to override the use of Bodi, e.g. Specflow.Autofac.

Are you saying you're not seeing the behavior described in this issue because you've specifically overridden the type of DI container being used? If so, would you mind sharing what you're using instead and linking to how you've overridden it in SpecFlow? (As well as a link to a location where you're calling ScenarioContainer.IsRegistered() for something you register at the Feature/TestThread-level.) If not, I think your project may actually be using Bodi, and you're being affected by the IsRegistered() behavior described in this issue 🙂 Just FYI.

@DeLaphante
Copy link

DeLaphante commented Nov 12, 2024

@DrEsteban - no worries 😅 - yeah you are right that Bodi was created for it but alot of folks use it as an extra dependency by adding the nuget package and using the ObjectContainer however the DI container is already built into Specflow itself. See the following screenshots and how I'm using it without any issues.

https://github.com/DeLaphante/CynkyAutomation/blob/master/CynkyAutomation/StepDefinitions/UI/Common/Common_UISteps.cs

image

image

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

No branches or pull requests

3 participants