Call for action: review/try new dependency plugin design

[gasparnagy]

We have quite many issues with our DI plugins for various reasons, and the different plugins solve the same problems differently that makes the maintenance harder.

With this motivation, I worked on a prototype to see the new concept.

• The prototype does not need changes in Reqnroll, so it can be tried out with any Reqnroll v2.3.0 project.
• The prototype contains a small sample that demonstrates the usage of different dependencies. The prototype and the sample has been created for Microsoft.Extensions.DependencyInjection (MSDI), Autofac and there is also an example that shows how people could configure dependencies with the built-in DI framework (BoDi) in the new model.
• The code of the prototype with the samples are at: https://github.com/gasparnagy/Reqnroll-DiPluginPrototype

I would like to ask you to review the concept (see details below) or even more, try it out in your own project! Any feedback is appreciated!

If the feedback is positive, the new design model will replace the existing plugins, but we will somehow make the existing plugins also available for a while to make the transition easier.

This is a long post, but I encourage you to scan it through to get a first impression and understand the concept. I also have a section at the end with the most important topics where I would like to get feedback.

Prototype structure

• The prototype contains 3 sample projects (MSDI, Autofac and BoDi) each of them are just a normal Reqnroll project with a few scenarios and step definition classes and the container configuration (Support/SetupTestDependencies.cs).
• The projects contain a ToReqnroll folder - these are the codes that would eventually be part of the Reqnroll codebase. This is what you need to copy to your own project if you want to test the new design.
• The core code extensions (ToReqnroll/Runtime) are stored only in the ReqnrollPlugins.DependencyInjection project and linked to the other projects to avoid duplication.

Sample test

• The sample test consist of 3 parts: a calculator test (Calculator.feature, CalculatorStepDefinitions.cs), a test that uses differently scoped dependencies (StepDefinitionsWithDependencies.cs, Scenario A, B, D, E in Feature1.feature and Feature2.feature) and a test that uses Reqnroll infrastructure (StepDefinitionsWithReqnrollDependencies.cs, Scenario C, F in Feature1.feature and Feature2.feature).
• The calculator test uses a calculator (ICalculator and Calculator) that has a dependency to ICalculatorConfiguration which have a test-only implementation: TestCalculatorConfiguration. The idea is that the calculator instance should be created per scenario (scenario scope), but the TestCalculatorConfiguration should be singleton (test-run scope).
• The differently scoped dependencies test basically uses a dependency in all 4 scopes that is available in Reqnroll: test-run (aka global), worker (aka test-tread), feature and scenario. It demonstrates the lifetime and the dispose behavior of these dependencies.
• The Reqnroll infrastructure is just uses different Reqnroll infrastructure (like scenario context) to demonstrate how they could be accessed.

Usage model in the new design

The dependency configuration model has been somewhat unified. For each DI framework, you have to define a class (SetupTestDependencies in the sample) and mark it with the [DependencyConfiguration] attribute. You need to use a base type, depending on the DI framework and override methods. E.g. use AutofacDependencyConfiguration for Autofac.

This class contains all dependency configuration details.

Optionally you can pass an "options" object (e.g. AutofacDependencyConfigurationOptions for Autofac) where some framework specific options can be set. The only interesting is the AutoRegisterBindingTypes (default: true) that controls whether all "binding" class should be automatically registered (in some of the old plugins this had to be done manually).

Usage model for Autofac

Configuration base class: AutofacDependencyConfiguration

For Autofac, the main method you need to override is the SetupServices method, where you can configure all dependencies (it receives a ContainerBuilder instance). There are different scope helper methods (ScenarioScope, FeatureScope, etc.), to define the lifetime of the particular objects.

// Register globally scoped runtime dependencies
containerBuilder
    .RegisterType<TestCalculatorConfiguration>()
    .As<ICalculatorConfiguration>()
    .TestRunScope(); // == .SingleInstance();

// Register scenario scoped runtime dependencies
containerBuilder
    .RegisterType<Calculator>()
    .As<ICalculator>()
    .ScenarioScope(); // == .InstancePerMatchingLifetimeScope(DependencyLifetime.Scenario.GetName());

The usage is just straightforward, no special consideration needed.

There is an alternative way of configuring the scopes: you can optionally override the SetupScenarioScope, SetupFeatureScope, etc. methods, to provide registrations right to those scopes. (The end result is the same.):

protected override void SetupScenarioScope(ContainerBuilder scenarioContainerBuilder, IObjectContainer scenarioContainer)
{
    // alternative way to register scenario scoped dependencies
    scenarioContainerBuilder.RegisterType<ScenarioDependency>()
        .As<IScenarioDependency>()
        .SingleInstance();
}

Usage model for MSDI

Configuration base class: MsdiDependencyConfiguration

The Microsoft's DI framework was originally designed to serve ASP.NET MVC apps and therefore limited in a few ways. There are two key limitations that impacts our usage:

1. It only supports two level scope hierarchy: "root" and "scoped" (nested scopes are practically not supported)
2. It does not support externally-owned services (so services that are not disposed by MSDI)

At the end the prototype shows a model where using test-run and scenario dependencies (the two most commonly used) is quite straightforward, but for the feature and the worker scopes you need special usage and also special usage needed for some Reqnroll infrastructure.

Just like with Autofac, you need to start by overriding is the SetupServices method, where you can configure all dependencies (it receives a ServiceCollection instance). For the two common scope, the usage is simple: use AddSingleton for test-run and AddScoped for scenario dependencies:

serviceCollection.AddSingleton<ICalculatorConfiguration, TestCalculatorConfiguration>();
serviceCollection.AddScoped<ICalculator, Calculator>();

For the others, you need to use wrapper classes both for registration and for usage. For example to use something in feature lifetime, you need to register it in two steps:

serviceCollection.AddScoped<IFeatureDependency, FeatureDependency>();
serviceCollection.AddTransient<FeatureScopeService<IFeatureDependency>>();

If you want to use this dependency, you need to depend on FeatureScopeService<IFeatureDependency>:

public class StepDefinitionsWithDependencies(FeatureScopeService<IFeatureDependency> featureDependency)
{
    [When("a feature dependency is used")]
    public void WhenAFeatureDependencyIsUsed()
    {
        // Feature dependency used via the Instance property of the wrapper
        featureDependency.Instance.Use();
    }
}

When accessing Reqnroll dependencies, some, like IReqnrollOutputHelper works just fine. However, for some other you need to change the usage. For example, you cannot directly depend on ScenarioContext (because MSDI would dispose it), but you have to depend on IScenarioContext instead. (Similarly with FeatureContext/IFeatureContext, TestThreadContext/ITestThreadContext). This is a limitation that we could later eliminate.

Using the interface instead of the class is enough for most of the cases, but if not there is always a backdoor: you can depend on ReqnrollServiceHandle<ScenarioContext> instead. This is a wrapper that protects the ScenarioContext from being disposed by MSDI. The usage is like: scenarioContextHandle.Instance.ScenarioInfo.Title.

Usage model for BoDi

Configuration base class: ReqnrollDiDependencyConfiguration

The dependency configuration with BoDi is also pretty straightforward. Here you can override any of the following methods: SetupScenarioScope, SetupTestRunScope, SetupFeatureScope, SetupWorkerScope and configure your dependencies as needed. The calculator sample config would look like this:

protected override void SetupScenarioScope(IObjectContainer scenarioContainer)
{
    scenarioContainer.RegisterTypeAs<Calculator, ICalculator>();
}

protected override void SetupTestRunScope(IObjectContainer testRunContainer)
{
    testRunContainer.RegisterTypeAs<TestCalculatorConfiguration, ICalculatorConfiguration>();
}

The usage is just normal, no tricks. Obviously the same for Reqnroll dependencies.

Implementation details

I have added quite some comments to the classes in the ToReqnroll folder, so I do not want to go through that here, just a few notes:

1. The concept is based on overriding the existing ITestObjectResolver interface (like the old plugins did), but this does not have to be done for each DI framework, but the implementation scans through the test project for a [DependencyConfiguration] attribute and uses that class. If there are multiple classes marked, there is an optional Priority setting that is used to pick one.
2. The class that has the [DependencyConfiguration] attribute must implement the IDependencyConfiguration interface (the base classes of the known DI frameworks do that). This is an extremely simple interface, it has basically only a single TryResolve method (and one for error messages). This might reduce the overhead of implementing a new configuration class for a new DI framework. You do not even need to make a Reqnroll plugin for that.
3. The implementations work similarly: they try to locate (or create) the scopes of the particular DI framework and then resolve the object from that.
4. The methods in the DI specific base classes that the user need to override are empty by default (in some cases even abstract), so forgetting to call the "base" method does not cause any problem.
5. The model has an error reporting part as well, that helps the user with meaningful error messages, e.g. they tell in the error message what to configure and how (try it by commenting out the ICalculator registration!)
6. The set of Reqnroll infrastructure objects that are exposed to the user are also unified: they are defined in the DependencyConfigurationOptions class that is the base class of all options class.
7. The MSDI implementation is more complex than the others because of the problems mentioned above. It is better to review the Autofac one first.
8. If there is no class with [DependencyConfiguration] attribute it falls back to the default behavior. We can later improve this in a way that if the plugin is used but the [DependencyConfiguration] attribute is not, we throw an error with some details on how to start.
9. I have used new names for some scopes: the "global" scope is called "test-run" and the "test-thread" scope is called "worker" ("feature" and "scenario" remained the same). This is to test the new names we once discussed with @Code-Grump.

Feedback questions, limitations

I tried to make the prototype "feature complete", although the code might need a few smaller adaption once it will be merged. You can give feedback about anything, but I am primarily interested to get feedback from the user perspective.

Some detailed feedback topics:

1. If the "annotated class with a base class" concept convenient. Do we need the attribute or just having a public non-abstract class in your project that implements an interface is enough?
2. Is it flexible enough to integrate it with the DI setup needs of your app? Do we need any other options?
3. Do the error messages helpful?
4. How to make the initial step easier (how the user will know that they need a class with a specific base class and attribute)? Shall we throw an error if the plugin is used, but no class has been tagged?
5. It currently searches for the configuration class only in the test assembly. Shall we also search in all configured binding assembly?
6. For the MSDI and the Autofac, I included overridable InitializeXXXScope methods (e.g. InitializeScenarioScope). These methods are called after the particular scope has been built (so you cannot config anymore), but before they get used. They could be used to initialize some dependencies in the scopes (e.g. initialize a cache in feature scope). Does that make sense? Do you have any good use-case for this?
7. If the new scope names, "test-run", "worker", work better than "global" and "test-thread"
8. General feedback about the names used by the public API (eg. DependencyConfiguration or SetupServices)
9. Using the MSDI (not well known) abbreviation for Microsoft.Extensions.DependencyInjection. (What should we use instead in the class names?)
10. Should we include the DI framework name in the base class, or should the base class always be called DependencyConfiguration (or DependencyConfigurationBase) in each plugins (in different namespaces of course)?

One limitation is that the logging support from @StefH is not yet integrated to the prototype. (Related issue: #484)

[gasparnagy]

Tagging users who had related issues recently: @304NotModified @olaviedoc @justhoma @13dante04 @padnevici @vinnyrose @StefH @sanjeev-chevireddy @mbhoek @ajeckmans @mcraa @rban-newday @e317855 @Antwane @aoletubo @AhmedElbaik @RicardoLans
@q42jaap @robertcoltheart

[304NotModified]

Cool stuff!

So why do we need an attribute and inheritance? That 's a bit strange IMO. I think that's feedback topic 1

Im also not sure if we should support multiple DependencyConfigurations (with ordering etc). In the past I needed a different servicecollection for another (specflow) feature. It that maybe the idea?

I assume that the abbreviation MSDI is temporary. I never heard of that abbreviation (:

Im on mobile so havent checked all code. Do we really need a regex? (Inside MsdiDependencyConfiguration)

We have quite many issues with our DI plugins for various reasons

Is one of the reasons, the lack of (unit) tests for the plugins?

It would be great if the new plugin is easy to unit test to prevent bugs.
I propose to add an extra interface so that we could easily test the assembly scanning (DependencyCustomizationsServices). See
IAssemblyTypeResolver in #475

[gasparnagy]

Thx for the valuable feedback!

So why do we need an attribute and inheritance? That 's a bit strange IMO. I think that's feedback topic 1

Good point. I did not think about that too much, but I think my decision was because "just having a class somewhere that implements an interface" feels a bit too implicit. But I'm open to get rid of the attribute. Would be glad to hear others options on this as well.

I'm also not sure if we should support multiple DependencyConfigurations

I think my original intention has been changed, so this can be removed. I was thinking on situations when you have configurations in multiple referred binding assemblies and you want to choose from that. But finally it searches in the test assembly only anyway, so this risk is not valid anymore.

Obviously this might be related to your first comment. Because if we want to support multiple, we need metadata for the selection so that would imply that we need the attribute.

I assume that the abbreviation MSDI is temporary. I never heard of that abbreviation

This is tough, because this library does not have a proper name. I was not coming up with MSDI (if you google for MSDI .NET you will see that it is being used time-to-time), but I also haven't heard about this before. Actually this particular naming question I wanted to include in the feedback list anyway. So if not MSDI then how should we call the base and the options class? MicrosoftExtensionsDependencyInjectionDependencyConfiguration, MicrosoftDependencyInjectionDependencyConfiguration, MicrosoftDependencyInjectionConfiguration? Or shall we just call the base class in all the different plugins as DependencyConfiguration (they will be in different namespaces of course)?

Do we really need a regex? (Inside MsdiDependencyConfiguration)

😎 The regex is used only to give better suggestions in the error message if possible. So basically the error wrapper tries to guess the problematic interface name from the exception that comes from the DI. But this is just a convenience, if it could not find anything it uses a generic placeholder. (The exception from the DI is always added as an inner exception of course.)

Is one of the reasons, the lack of (unit) tests for the plugins?

That is definitely part of it, although the reason for designing the new model was not that, but that the usage, the functionality and the inner structure of them were quite different and not easily extensible. (The concept of expecting the user to attribute methods and guessing the proper signature (which might even change with versions) for it was a very bad idea (of me)... Mea culpa. 😁)

It would be great if the new plugin is easy to unit test to prevent bugs.

Definitely. Once it gets into the codebase we need to cover this with unit tests. Basically this samples in the repo serve only as a basic "smoke test", so we would probably include them to the Reqnroll.SystemTests but we need to have other tests as well, so the code might need to be modified for better testability of course. Your PR is noted 🙏!

[304NotModified]

Or shall we just call the base class in all the different plugins as DependencyConfiguration (they will be in different namespaces of cours

Same name, different namespace sounds good. Also the plugins will be in different nuget packages? So no clash if even in the same namespace. (Not saying that's a good idea)

The regex is used only to give better suggestions in the error message if possible

If there are unit tests for these cases we could maybe fix it without regexes. Regexes give sometimes some security headaches and avoiding is sometimes better.

Would be glad to hear others options on this as well.

Yes me too :)

[olaviedoc]

I have tested your prototype with in our codebase using MSDI. I can confirm that the singleton services are now disposed of properly when the test run ends (issue #465).

Feedback

Step context

Since the StepContext isn't part of the IScenarioContext interface, we had to use ReqnrollServiceHandle<ScenarioContext> as parameter type for our hooks.

Would it be possible to add the StepContext to the IScenarioContext (or if you make it possible to use the ScenarioContext again)?

Old code

[AfterStep]
public async Task AfterStep
(
    ScenarioContext scenarioContext
)
{
    var scenarioStepContext = scenarioContext.StepContext;
    Debug.WriteLine($"After step: {scenarioStepContext.StepInfo.Text}");
}

New code

[AfterStep]
public async Task AfterStep
(
    ReqnrollServiceHandle<ScenarioContext> scenarioContextHandle
)
{
    var scenarioStepContext = scenarioContextHandle.Instance.StepContext;
    Debug.WriteLine($"After step: {scenarioStepContext.StepInfo.Text}");
}

MsdiDependencyConfiguration

Some of our services use the options pattern. That means that we need access to configuration (IConfiguration), in order to add the options to the service collection. See code bellow.

private static IServiceCollection AddWebAutomationSettings(
    this IServiceCollection serviceCollection,
    IConfiguration configuration
) =>
    serviceCollection
        .Configure<WebAutomationOptions>(
            configuration.GetSection("WebAutomation")
        );

Currently we're getting the configuration by a bit of a hack (creating an application builder and then getting the configuration from it):

var builder = Host
    .CreateApplicationBuilder();
serviceCollection
    .AddWebAutomationSettings(
        builder.Configuration
    );

I'm wondering if there's a possibility to provide the IConfiguration to us somehow (like a property or an argument, so we don't have to do the application builder trick). Or if anyone has a better alternative to do configuration, but still keeping the options pattern.

Answers to the feedback questions

1. To us I think the base class is enough (no need for the attribute)
2. I think it is flexible enough. See separate note about the step context above.
3. If I remember correctly I lacked the context for when the type resolving failed (for instance MSDI gives that info on this form: Unable to resolve service for type 'IServiceC' while attempting to activate 'ServiceB', while attempting to activate 'ServiceA'.).
4. Yes, an exception/error seems like an appropriate way to communicate this.
5. To us the current test assembly is enough, feels like a reasonable limit.
6. We have no use for it at the moment, but maybe it will be usefull to others/in the future?
7. I'm new to Reqnrol so I have no previous experience with the old names,
   but I think "test-run" makes more sense when "global" (if the lifetime is tied to the test-run).
   The name "worker" doesn't say much about the lifetime by itself (how long does the worker live?), but if you document it somewhere that is easily found I see no problem with it.
8. Maybe something a long the lines of ConfigureServices like in StartupBase.ConfigureServices
9. What about only having the full name in the namespace and no prefix/suffix for the classes, ie. ReqnrollPlugins.DependencyInjection.MicrosoftDependencyInjection as namespace and DependencyConfiguration as class name in that namespace (might require adjusting a few names in order to not have name conflicts)? I'm also fine with having Msdi as prefix if that makes things easier/better.
10. See answer to question 9 above, but no strong opionions really. Most important to be consistent and trying to avoid breaking changes in future releases I think.

[gasparnagy]

Thx.

Since the StepContext isn't part of the IScenarioContext interface, we had to use ReqnrollServiceHandle as parameter type for our hooks.
Would it be possible to add the StepContext to the IScenarioContext (or if you make it possible to use the ScenarioContext again)?

Yes, definitely. Thx for noting this. There are a couple of things that are not on the interface. We have to put them on indeed.

Some of our services use the options pattern. That means that we need access to configuration (IConfiguration), in order to add the options to the service collection.

Good to know. I'm not familiar with this pattern, but it seems that there is a special set of magic specific to ASP.NET hosting that combines all details to an IConfiguration (see here). This is not even in the Microsoft.Extensions.DependencyInjection package that we use but in the Microsoft.Extensions.Hosting package, so we don't have a chance to provide it. So currently I don't see a better option then what you did, but maybe someone else has.

3. If I remember correctly I lacked the context for when the type resolving failed (for instance MSDI gives that info on this form: Unable to resolve service for type 'IServiceC' while attempting to activate 'ServiceB', while attempting to activate 'ServiceA'.).

Could you please double check that? We just wrap the exception but do not hide anything (at least that was the plan...).

[StefH]

I've just a quick remark / feedback. I did some quick prototyping and testing with your project, but could not get it the ILogger to work with IReqnrollOutputHelper.

The IReqnrollOutputHelper is only available in the InitializeTestRunScope, but not when the Calculator is registered.
So this will not work:

public class Calculator(ILogger<Calculator> _logger, ICalculatorConfiguration _calculatorConfiguration) : ICalculator

Also it seems that the ambient Reqnroll context accessor is not completely accessible in every stage of the test. But probably I missed some knowledge on this part.

[gasparnagy]

Thx. The logger stuff is still to be done, indeed.

Also it seems that the ambient Reqnroll context accessor is not completely accessible in every stage of the test. But probably I missed some knowledge on this part.

Could you please give an example of what you want by that?

[Code-Grump]

My initial thoughts (a more in-depth review is deserved and pending):

Dependency Registration

Requiring an attribute is probably better than requiring inheritance, because it's directly descriptive of the intent. You can't "accidentally" pick up an unintended behaviour by inheriting from a class or implementing an interface. If we're looking for an attribute, inheritance or implementation of an actual interface should probably not be necessary; if we're invoking by reflection we are effectively doing so by convention and therefore can free users from the need to include the interface (or indeed for us to define it).

However, this leads me to the thought that perhaps it's due time we created a proper "startup" routine for Reqnroll.

Reqnroll Startup

In the same way that a console app has an entry point in Program.cs and ASP.NET bootstraps its configuration in Startup.cs and more recently in Program.cs , is this prototype an opportunity to create a similar "before test-run" hook that provides the option to influence Reqnroll's execution pipeline?

Using just BoDi:

public class ReqnrollStartup
{
    public void Configure(ReqnrollBuilder tests)
    {
        tests.Services.RegisterInstanceAs(new MyService(), dispose: true));
        // etc.
    }
}

Or with MSDI

public class ReqnrollStartup
{
    public void Configure(ReqnrollBuilder tests)
    {
        tests.Services.UseMicrosoftDependencyInjection(services =>
        {
            services.RegisterSingleton<MyService>();
            // etc.
        });
    }
}

It's a system that's invoked by convention - just having the type with the name and the signatures is sufficient.

The advantages of this are longer-term than simply DI, giving space to register all kinds of potential customisations. It opens the possibility of making customisations without the need for creating full plugins. Any time ordering matters, we now have a trivial place to register a sequence/chain of components that makes order an explicit part of registration.

Perhaps most promising of all, this class could be invoked by tooling or other systems to gain access to a more complete model of the user's intentions. Want to supply configuration from environment variables? Our generators and IDE integration can invoke the same configuration method to build their own internal models.

Microsoft Dependency Injection Limitations

Since there's a "special" syntax required for the registration of objects, we should consider creating new extension methods to encapsulate this help extend our terminology to those using it:

services.RegisterTestRunScoped<MyService>();
services.RegisterFeatureScoped<MyService>();
services.RegisterWorkerScoped<MyService>();

It's an opportunity to add documentation in a place most users will encounter it (intellisense) to help explain what each registration method does and what scopes exist and helps "smooth over" the pain-points.

[olaviedoc]

I really like your suggestion, it gives much more control and flexibility (and it's a pattern proven to work by ASP.NET etc.).

[olaviedoc]

One thing though; we will probably run it to problems with the new suggested extension methods (AddTestRunScoped et al) since we are doing the service registrations in another assembly (TestCases) that has nothing to do with Reqnroll (it doesn't and shouldn't have a dependency to it). Using ReqnRoll is only one way to exercise the test cases in the TestCases assembly, another way we use is to have another MSTest assembly running the tests in the TestCases assembly. The TestCases assembly provides an extension method AddTestCases (for registering it's test cases etc in the ServiceCollection) which is in turn called by either the Reqnroll project or the MSTest project, so it needs to work both in a Reqnroll context as well as in an MSTest context.

[Code-Grump]

I don't believe it's reasonable to have a set of dependency registrations produce a compatible suite of services between two different service contexts when one of those contexts has to work alongside the Reqnroll infrastructure and its specific lifecycle. If your services can run without a dependency on the Reqnroll library, they probably shouldn't be included in the Reqnroll service container and isolated to their own service container.

It still breaks a lot of expectations to do this: there are no nested scopes with Microsoft's DI implementation and we created a scope for every test run. If your service registers a singleton, you'd get one for the whole test run.

[gasparnagy]

In the same way that a console app has an entry point in Program.cs and ASP.NET bootstraps its configuration in Startup.cs and more recently in Program.cs , is this prototype an opportunity to create a similar "before test-run" hook that provides the option to influence Reqnroll's execution pipeline?

I like this idea. I will give it a try.

Since there's a "special" syntax required for the registration of objects, we should consider creating new extension methods to encapsulate this help extend our terminology to those using it

I agree.

I don't believe it's reasonable to have a set of dependency registrations produce a compatible suite of services between two different service contexts when one of those contexts has to work alongside the Reqnroll infrastructure and its specific lifecycle.

It depends. Most of the applications (not all, of course) have two-level DI lifetime: like with simple ASP.NET like dependency models, where you have global singletons and request lifetime. These can be easily mapped (in most of the cases) to "test run" and "scenario" scope.

The other scopes that are provided by Reqnroll ("worker", "feature") are only senseful for test-related dependencies anyway.

So what I want to say that we can and should provide helper methods for configuring the DI with a Reqnroll-specific lifetime (e.g. RegisterTestRunScoped<MyService>()), but we should also clearly document what these do and also we should keep the practice of mapping the "default" lifetime to the "scenario" (because this is the safest anyway), and the "singleton" lifetime to the "test-run" (as this is anyway a natural match).

For any other more complex case, I agree to you and it should be the caller's responsibility to do the mapping between their own lifetimes and the testing ones.

[RicardoLans]

I really like the idea of having an entry point like startup where we can easily register services.
One side-note, also consider an Async method to register services, since we use Microsoft Playwright and they only support async when creating their types.

[Romfos]

I think that there is a lot of good suggestions in this discussion.
My proposition is to follow app builder pattern that you can see in aspnet like that:

public static class Program
{
    public static async Task Main(string[] args)
    {
        var builder = new SpecflowApplicationBuilder();

        builder.Configuration.UseStandardReqnrollJsonConfiguration();
        builder.UnitTestsPlatform.UseMsTestAdapter();
        builder.Logging.UseDefaulLogger();
        builder.Services.AddSingleton<IMyCustomService>();

        builder.UseCustomPluginFromExternalLibrary();

        builder.Features.AddFeaturesFromExecutingAssembly();
        builder.StepsDefinitions.AddStepsFromExecutingAssembly();

        var specflowApplication = builder.Build();

        await specflowApplication.RunAsync();
    }
}

potential builder:

public sealed class SpecflowApplicationBuilder
{
    public SpecflowConfigurationBuilder Configuration { get; } = new();
    public UnitTestsPlatformBuilder UnitTestsPlatform { get; } = new();
    public SpecflowLoggingBuilder Logging { get; } = new();
    ...
    SpecflowApplication Build();
}

public sealed class SpecflowApplication
{
    Task RunAsync();
}

Rules:

1. Static classes\methods\properties\e.t.c are banned (maybe marked as deprecated on the first version). You can override any services via di and public interface
2. Extension via DI container and extension methods to this builder like builder.UseCustomPluginFromExternalLibrary(arg1, arg2)
3. No magic like automatic assembly scanning, e.t.c. For extensibility should be syntax that use should explicitly call like:

builder.Scenarios.AddScenariosFromExecutingtAssembly();
builder.CustomStepTranformations.Add<MyCustomStepTransformation>();

as the result you have only what you specified in builder => faster startup & run without not needed services. You should check your build and be sure that exactly you are using and where

4. Code first approach. Now you need to google all settings names. Would be better to get them from intellisense.
   You can still use json config via:

builder.Configuration.UseStandardReqnrollJsonConfiguration();

but it should be optional way as any other config provider. The same for Logger, e.t.c. Maybe these should be remove from "core" library and provided as external library for those who needed it

5. App should have minimal amount and reasonable defaults

var builder = new SpecflowApplciationBuilder();
builder.Scenarios.AddScenariosFromExecutingtAssembly();
await builder.Build().RunAsync();

should work and work really fast

My private opition about reqnroll design in general:

I think that for years specflow & reqnroll get tons of "features" that I really never (or rare) use like custom json parsers\custom di\multiable adapters for tests\step args transformations\plugin system\feature+scenario contexts\report generators\source generators\cucumer messages\e.t.c. Tons of them!!

From my point of view would be better to split (or reimplement) it like

1. Reqnroll nuget that allow just run scerarios and support step definition
2. Reqnroll.SpecflowCompatibility nuget . Plugin that "returns all shit back"
3. Reqnroll.Configuration.ReqnrollJson nuget. Extends to load config from reqnroll.json. only for those who need it
4. Reqnroll.ExcelDataPlugin nuget. That add support for excel data source. again, only for those who need it
   e.t.c

Features that should be out of the box:

1. App builder
2. Default DI abstraction
3. Default logging abstraction (default console output)
4. Default configuration abstraction (via code only, no default providers from external sources)
5. Ability to register feature files
6. Ability to register step definition classes
7. Maybe hooks pipeline?? (like BeforeScenario)
8. List of public interfaces that plugins could implement and register in DI to provide extra features
   That's it!

As the result it should be small\easy to support\fast to run library

[clrudolphi]

This comment may be a seem a bit primitive, but I think it would be helpful to get in writing the rationale for, and the design objectives of, having a pluggable DI system.

I will grant that developers of bindings may need to arrange a container that is consumed by the SUT. But is it necessary or even desirable that it be the same container mechanism used internally by Reqnroll?

What facilities or abstractions provided by the Reqnroll runtime are needed for consumption by the SUT? In other words, are there any reasons for the SUT container and the Reqnroll container to touch or intermix?

How or why would it be necessary for users of Reqnroll to swap in a different container (thus replacing BoDi) for consumption by the Reqnroll runtime?

[Code-Grump]

I think I hold similar reservations. My position is that:

• Our DI system should be only for Reqnroll components, ones we've added ourselves or ones added by plugins.
• The SUT has no business being directly registered in this container.

If users want to do this for some exotic reason, they're absolutely free to, but we shouldn't encourage it as some kind of default by providing a swappable architecture so they can fall into a trap of their favourite flavour. This is an advanced scenario. We should provide guidance, but put massive warning signs at every opportunity.

[hotchkj]

Having come to this issue from queries about the current ReqNRoll Microsoft.Dependency.Injection code, I tend to agree with the above.

Yes, near-everyone uses some flavour of DI. Yes, ReqNRoll undoubtedly benefits from its own DI approach to build its internals.

Also yes, many users will want to init their tests efficiently without a lot of boilerplate to consume their particular DI.

The way I see it, this should be a layered approach.

Layer 1: ReqNRoll does what it needs to do, offering specific interfaces that can be consumed by anything downstream in actual tests. Maybe this is just BoDi.

Layer 2: N plugins offering boilerplate that means it's easy to init tests in the chosen pattern, and linking between the layers. Maybe some success criteria would help. To take xunit + MS DI, my personal preference, if I have a test:

[Binding]
class MyStepDefinitions
{
    public MyStepDefinitions(
        IReqNRollInterface, // From ReqNRoll internals, exposed for controlling actual Gherkin behaviour
        IMyOwnInterface,    // From my own DI, instantiated correctly for my test style e.g. usually one DI collection per test
        ITestOutputHelper)  // From XUnit
    {
    }
}

What a consumer of the above really wants is that it 'just works'. I don't really care how I get these interfaces to my tests, I care that they're the right lifetimes and expectations - that's the value-add of plugins in the first place.

This feels like what MS DI's IServiceProvider abstraction was designed for (not that you must use this, but something similar in concept). You can look up into several containers and resolve from the right source. Need something from XUnit? XUnit's container. Need ReqNRoll's exposed interfaces? The ReqNRoll container. Each container has the lifetime for its appropriate purpose. You can also constrain the containers to expose only what you want - none of the ReqNRoll interfaces that you use and don't want consumers to have access have to be in the container the plugin has access to.

The plugin has the role of designing the right abstractions for the choice of testing framework & DI framework, and resolving from the right sources.

My two pence.

[gasparnagy]

Having come to this issue from queries about the current ReqNRoll Microsoft.Dependency.Injection code, I tend to agree with the above.

Yes, near-everyone uses some flavour of DI. Yes, ReqNRoll undoubtedly benefits from its own DI approach to build its internals.

Also yes, many users will want to init their tests efficiently without a lot of boilerplate to consume their particular DI.

The way I see it, this should be a layered approach.

Layer 1: ReqNRoll does what it needs to do, offering specific interfaces that can be consumed by anything downstream in actual tests. Maybe this is just BoDi.

Layer 2: N plugins offering boilerplate that means it's easy to init tests in the chosen pattern, and linking between the layers. Maybe some success criteria would help. To take xunit + MS DI, my personal preference, if I have a test:

[Binding]
class MyStepDefinitions
{
    public MyStepDefinitions(
        IReqNRollInterface, // From ReqNRoll internals, exposed for controlling actual Gherkin behaviour
        IMyOwnInterface,    // From my own DI, instantiated correctly for my test style e.g. usually one DI collection per test
        ITestOutputHelper)  // From XUnit
    {
    }
}

What a consumer of the above really wants is that it 'just works'. I don't really care how I get these interfaces to my tests, I care that they're the right lifetimes and expectations - that's the value-add of plugins in the first place.

This feels like what MS DI's IServiceProvider abstraction was designed for (not that you must use this, but something similar in concept). You can look up into several containers and resolve from the right source. Need something from XUnit? XUnit's container. Need ReqNRoll's exposed interfaces? The ReqNRoll container. Each container has the lifetime for its appropriate purpose. You can also constrain the containers to expose only what you want - none of the ReqNRoll interfaces that you use and don't want consumers to have access have to be in the container the plugin has access to.

The plugin has the role of designing the right abstractions for the choice of testing framework & DI framework, and resolving from the right sources.

My two pence.

I might be wrong, but I think this prototype does exactly what you described: it forwards the Reqnroll dependencies from body and manage the lifetime of your own ones.

Or was there anything in the prototype that would contradict to this?

[TimMurphy]

Word on warning. Project won't compile with Reqnroll 2.4.1, 2.3.0 works.

[TimMurphy]

I've deleted the post that I didn't think it worked correctly. It works as expected. Thank you.

[TimMurphy]

Any expected release date? What can I do to help with this issue?

[gasparnagy]

I was pretty busy with other things and therefore this was delayed. To be honest, most probably this is something for 2026. The help is appreciated. I wonder what would be the best approach to address this. Maybe we could first add these as temporary preview packages (e.g. with names, like Reqnroll.Autofac.Preview, so that we don't disturb the existing users and get closer to a real solution. If you feel you can help to kick this off, that would be fine.

[TimMurphy]

Apolosies for tardiness in replying, life happens.

I agree with your suggestions of preview packages.

This will be my first significant contribution to an open source project so I'll need some hand holding.

Where do I begin? What branch/commit do I start with? Why is this stuck on v2.3.0?

[gasparnagy]

Apolosies for tardiness in replying, life happens.

I agree with your suggestions of preview packages.

This will be my first significant contribution to an open source project so I'll need some hand holding.

No prob.

Where do I begin? What branch/commit do I start with? Why is this stuck on v2.3.0?

It is at v2.3.0, because we haven't updated that yet. No particular reason.

Maybe you can start by trying to "upgrade" the code in my temporary repo to v3.2: make a fork of my repo, clone the forked repo for yourself, try to upgrade it to v3.2 and send a pull request to my repo. This will be a good first practice and nothing can go wrong. I am happy to help if you get stuck.

[TimMurphy]

I'll start working on it later this week.

…

On Mon, 1 Dec 2025 at 21:42, Gáspár Nagy ***@***.***> wrote: Apolosies for tardiness in replying, life happens. I agree with your suggestions of preview packages. This will be my first significant contribution to an open source project so I'll need some hand holding. No prob. Where do I begin? What branch/commit do I start with? Why is this stuck on v2.3.0? It is at v2.3.0, because we haven't updated that yet. No particular reason. Maybe you can start by trying to "upgrade" the code in my temporary repo to v3.2: make a fork of my repo, clone the forked repo for yourself, try to upgrade it to v3.2 and send a pull request to my repo. This will be a good first practice and nothing can go wrong. I am happy to help if you get stuck. — Reply to this email directly, view it on GitHub <#472 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADFDDRW72GXKDACTKX4HLD37QLP5AVCNFSM6AAAAABXMC7EFSVHI2DSMVQWIX3LMV43URDJONRXK43TNFXW4Q3PNVWWK3TUHMYTKMJSGMYDONA> . You are receiving this because you commented.Message ID: ***@***.***>

[craigfowler]

I realise I'm late to this (I took a long break from Open Source) but I'm very interested in Reqnroll DI, mainly because I maintain a library which hooks into all of that.

A long time (7y) ago, I forked (then ended up heavily reworking) BoDi to use it as the container for my project. I'm looking into producing a v2, based on the lessons learned from v1 and I'm presently headed down the MSDI route. Things which have come up in my own thinking:

• MSDI doesn't support nested scopes or named scopes, you must use a different container to get that
• BoDi has a unique (AFAICT) feature that it can auto-register (as scoped) a dependency which has not yet been registered, this is useful in Reqnroll for things like custom context objects (just models) which may be passed around a scenario's bindings

MSDI on the other hand comes with the huge benefit of the ecosystem around IServiceCollection and third party authors who write .AddXyz() service collection extension methods.

Given all of that, whilst I like the idea of Reqnroll switching to MSDI, really all I care about is the IServiceCollection interface as an extension point. Something which a lot of the third-party DI containers have done, is to provide an adapter for their own container, which presents it as the IServiceCollection interface. This means that the regular MSDI service collection methods may be used to register services (great for the ecosystem) but we don't give up any of the functionality of the full-featured third party DI container.

To me, this approach (go for a 3p full-featured container, but allow interaction via an adapter to IServiceCollection) would be a big win. I'm not too sure if this aligns with your collective vision for Reqnroll & DI.

One other idea to throw into the mix; if you're not aware of it - is Scrutor: https://github.com/khellang/Scrutor. That library adds "bulk register all types in an assembly" to MSDI. That might be useful as a solution/alternative to the "resolve unregistered types" functionality that's unique to BoDi.

[gasparnagy]

Thx for the note about the IServiceCollection adapter... That's worth considering.

Scrutor: Yeah, I guess that's a good hint to those who use Reqnroll with MSDI, thx.

[craigfowler]

I've got a few more thoughts (hastily written on a morning commute). Apologies for typos and the like.

Reqnroll includes a "Feature context" which is state that is shared per-feature. As I understand it, because it gets its own DI scope which is a parent of the Scenario scope, any mutable parts of that context which are mutated by one scenario after visible to the other scenarios in the same feature. What's the intended use-case for this and how prevalent is its use? Unless I'm missing something, it makes me wonder about whether it's a good thing to keep around? Scenarios/tests which can share state/be affected by one another seems like a test anti-pattern to me. If it's for 'common state set-up' then Cucumber already has functionality for that in 'Feature Background'. Using feature background, each scenario receives identical state, but each is a fresh copy of that state.

If we could remove the feature scope (not sure how controversial this is) then I think we'd also no longer require nested scopes at all. That would make it much easier to make MSDI the default container for Reqnroll, via the MSDI abstractions package. Users could use the actual MSDI impl if they want, or could install a different container which provides an IServiceCollection and IServiceProvider adapter (most of the popular ones do) using their chosen container's own functionality.

EG: In your "configure services" method, you'd use Autofac's own serviceCollection.AddAutofac() method, and immediately the container is switched to Autofac. Reqnroll needs to know nothing of it, because the ecosystem is all based around using the MSDI abstractions.

[gasparnagy]

What's the intended use-case for this and how prevalent is its use?

We have a separate discussion about this: https://github.com/orgs/reqnroll/discussions/468

Let's not get into that discussion in this thread, but in essence people use it for caching and not for mutated value.

If we could remove the feature scope (not sure how controversial this is) then I think we'd also no longer require nested scopes at all.

We also have the test-thread (or test worker) scope as well, that is again mostly used for caching, so it is not only the feature scope that causes the complication.

We have had plenty of discussions about whether these scopes are really necessary or not (again, I don't want to go into details in this thread), and the consensus was that yes, we could have avoided that, but since those are so deeply integrated to Reqnroll and the usage that it is not really feasible to get rid of them.

Still, this does not necessarily stopping us to use IServiceProvider as a suggested interface for custom DI solution integration. For example I can imagine a solution, where we say: "you can use custom DI, but then you cannot use feature context and test-thread context as a dependency scope" (you can still use them as a standard name-value dictionary if super necessary). But the feature context causes problems with the MSDI implementation only, not the interface - IServiceProvider could work fine even with Autofac that has no issues for the feature scope.

The IServiceCollection is slightly different though, because people might want to choose a more powerful DI (like Autofac), because they want to use the power of dependency configuration options that library provides and don't want to stick to the pretty basic options you get with IServiceCollection. But for some basic usage patterns it could be sufficient, still.

[craigfowler]

OK, cool. I'd not realised the scopes situation was a sore topic. No more about that 🫢

When using IServiceCollection with more full-feature DI containers (like Autofac, which I use in my day job project) the pattern tends to look a little like this:

public void ConfigureServices(IServiceCollection services)
{
    services.AddAutofac(ConfigureAutofacServices);
    
    // Add more services using the IServiceCollection API if you'd like.
    // The underlying DI container is now switched to Autofac because of the logic above.
    // The app (overall) didn't have to know anything about Autofac, because it was coded
    // only to the MSDI Abstractions standard.

    // Registrations made here and registrations made via ConfigureAutofacServices
    // may happily coexist.
}

void ConfigureAutofacServices(ContainerBuilder builder)
{
    // Register anything you want, using Autofac's ContainerBuilder API.
    // Here, you have full access to Autofac's functionality.
}

So, what I'm getting at is that if other containers provide similar functionality (when adding them to the service collection, they essentially replace the whole impl with themselves), then Reqnroll doesn't need any container-specific logic at all. All it needs know/care about is IServiceCollection and IServiceProvider. It's up to the consumer to pick/switch containers if they want.

[craigfowler]

Right, I've tried the Reqnroll DI prototype out with the extension I'm authoring. It would seem that this architecture for replacing the DI container wouldn't be able to coexist nicely with any other Reqnroll plugins which customize dependencies.

My understanding of why

Here's a quick sketch of the architecture as I see it. I've used Autofac as the example but I believe it would be true for MSDI as well.

1. The Autofac DI plugin and any other plugin receive the same CustomizeGlobalDependencies hook from the main Reqnroll library
2. Any DI registrations added there are sent to the underlying BoDi container which is inside Reqnroll
3. When tests run, the container used to resolve the tests (features, scenarios & dependencies) is the replaced Autofac container
4. But, as a consequence, any other registrations made into the BoDi container via CustomizeGlobalDependencies (aside from the replacement container) are now ignored because the BoDi container isn't being used to resolve anything

The result (in my case) is a resolution error when my tests run, because the dependencies introduced by my plugin haven't been registered with Autofac.

Where you can take a look

I've performed a trial integration with the Reqnroll in an experimental branch, you can see it here: https://github.com/csf-dev/CSF.Screenplay/tree/experiment/reqnroll-di-containers. The three ReqnrollPlugins.Xyz directories are just the source code from the prototype you'd created. The CSF.Screenplay.Reqnroll directory contains my runtime plugin for Reqnroll. The place where I integrated the new DI prototype logic with some tests is Tests/CSF.Screenplay.Reqnroll.Tests. If you were to build the solution and run just the tests in that directory then:

• In the master branch they'll pass
• If you were to check out commit 7f91e122735a9d7bc64da61d61786aa570c6944e then they'll pass; in this commit I've integrated with the "no op" BoDi DI plugin (IE: leave the DI container alone)
• If you check out the head of the experiment/reqnroll-di-containers branch - commit 73974099c0f49d3b995153289d19029859ac011f - then they will fail with the resolution error

Aside from the stuff mentioned above, if you want to see my DI registrations they are in the class ScreenplayServiceCollectionExtensions, which is executed from the plugin by ScreenplayPlugin.OnCustomizeGlobalDependencies (via another small extension method named AddScreenplayPlugin which is in ServiceCollectionExtensions in the same directory).

A note about running the other tests

I'd advise just running tests in Tests/CSF.Screenplay.Reqnroll.Tests - they're the only relevant ones here. If you try running the others then:

• You'll also need npm and to have performed an npm ci in the CSF.Screenplay.JsonToHtmlReport.Template/src directory
• The test suite includes a bunch of Selenium-driven web browser tests, so it'll cause a whole bunch of browser windows to appear and disappear automatically

Ideas to fix this

I've got some (very high-level, back of napkin) ideas as to how you could resolve this issue. The core problem being "other plugins would be adding their registrations to a container which is never used". I'm afraid, I think they're all breaking changes.
None of these are fully fleshed out in my mind, so they might explode on contact with real code etc. I've just thrown them out as-written in case they're any help.

Add a container-replacement hook

In this idea, you'd add a new customization/plugin hook which is executed before customize global dependencies. Why? Because we can't be certain of the order in which plugins are loaded and executed.

The new hook is only for the purpose of replacing the DI container. Then, for each of the CustomizeXyzDependencies hooks, instead of presenting the plugin with a concrete ObjectContainer to add their registrations, present the plugin with an abstraction which exposes only registration functionality (not resolution). If the container has been replaced then the impl behind that abstraction would be one provided by the DI-container-replacement plugin (an additional concept for the plugin). It means other plugins would be registering their deps with the replaced container, and not still trying to register with redundant BoDi container.

Add a way to 'port BoDi registrations to another container

In this idea, there's quite a bit more work added for each DI plugin. It's based around an idea which comes from Microsoft, related to the way in which MSDI works. Note that the container in MSDI is IServiceCollection - it really is a collection of registrations: ServiceDescriptor. It's conceptually the same technique which Autofac use, if in a normal app, you use Autofac.Extensions.Di to replace a stock MSDI ServiceCollection. What the Autofac container does when it builds its service provider is to "scoop up" all the registrations which were made in the MS ServiceCollection and add them as first-class Autofac registrations.

If you were to provide a similar mechanism, you could allow DI-container-replacement plugins to scoop up any registrations already made in the BoDi container (by other plugins, which don't know the container's being replaced). That DI replacement plugin would be responsible for re-registering anything already-registered in BoDi, within its own container.

Switch to MSDI by default

This is probably the most radical idea, and the most-breaking of them, but I also think worth considering because 'it takes a lot of the work off your hands'. In this idea, Reqnroll removes BoDi as the default container entirely (it could still exist, within). All DI registration/resolution APIs would need to switch to IServiceCollection and IServiceProvider instead of the BoDi equivalents. By default, the container which is used by Reqnroll would be an MSDI service collection. There are some consequences:

• Negative: Out of the box, you lose support for Feature (nested) DI lifetime scopes
• Negative: A very wide-reaching breaking change
• Positive: You might not need to create DI replacement plugins at all!
• Positive: You're future-proofed and integrate more seamlessly with the DI ecosystem

That loss of nested scopes could be resolved with a one-line addition to test projects. A .AddBoDi() extension method, for example, which switches the DI container in-use back to BoDi would give people back nested DI scopes.

The reason you might not need to create DI replacement plugins under this regime is that the DI container authors already did it for you. For example, just by installing Autofac.Extensions.DependencyInjection from NuGet, the developer can add to their DI configuration .AddAutofac() and now the container is switched to Autofac. The Autofac DI takes care of 'porting any registrations made with the MSDI abstraction into being first-class Autofac registrations. So, there's very little work for Reqnroll to do here. Any existing or future DI container which works with MSDI can be dealt with the same way.

Basing Reqnroll on the MSDI abstractions also means that (IMO the majority of) authors of additional services will integrate neatly with Reqnroll without further work. The ecosystem is headed in the direction that authors assume an app is going to expose the MSDI abstractions. Even if they're not using the MSDI impl, the abstractions will be there, so new deps/services can be packaged up in an public static IServiceCollection AddMyDependencies(IServiceCollection services) wrapper, and added to an existing app with just one extension method call.

I think there might end up being a little to do, because (probably) extension methods would be needed for IServiceCollection for things like registering services with different-named scopes. So, DI plugins might not go away entirely. For example an Autofac plugin might need a way to indicate to Reqnroll that it supports multiple, named DI scopes (well, Autofac calls them "tags" not names). On the other hand, MSDI does not (one scope only), so it should probably throw NotSupportedException if you try to use it to register feature-scoped services, for example.

[craigfowler]

I forgot to point out. You'll notice that all my DI registrations are written in MSDI syntax. I think I mentioned a service collection adapter in the past. This is the one I'm using for Screenplay/Reqnroll: https://github.com/csf-dev/CSF.Screenplay/blob/master/CSF.Screenplay.Reqnroll%2FServiceCollectionAdapter.cs

It's rather rudimentary and it could probably use a lot of improvement. It works for the very narrow usage I'm dealing with though.

[gasparnagy]

@craigfowler Thx for the input, this is valuable.

I was also thinking on some ideas similar to yours. Having a "container-replacement hook" might be needed, indeed. The "scooping up" is also something I considered, but in a form of "fallback" (if the resolution of the configured custom container fails, we try from BoDi as well). I would need to investigate your example a bit more in detail to understand why this fallback was not working.

For "Switch to MSDI by default", I think one key challenge is that Reqnroll itself also uses the DI scopes, so loosing the feature and the test-thread scope would cause quite some extra work (especially the test-thread one, that is used by 20+ Reqnroll components). Or did you consider to have better separation for the dependencies for the end-users (binding class instances and whatever they configure) and the Reqnroll internal dependencies? I don't know how easily this is possible, but using the MSDI only for the end-user dependencies is something that sounds more feasible.

[gasparnagy]

The more I think of it, the better I like the direction, that for user-customizable DI integrations, we should only support 2 scopes: global and scenario. The feature and test-thread scopes are anyway used by the end-users only for caching, but that could be solved somehow else as well (e.g. people can put their values to be cached to FeatureContext which anyway has a key-value collection), we don't really need a DI scope for it.

Internally, for Reqnroll we can still keep the multiple BoDi containers (because it would be too hard to get rid of them at least for now), but this would greatly simplify the external DI integration. And would be much more aligned with how ASP.NET deals with the DI containers (global + request scopes), so would me more familiar for the users.

I think this is in line with what @craigfowler suggested and also what @Code-Grump mentioned earlier.