reqnroll / Reqnroll

Open-source Cucumber-style BDD test automation framework for .NET.
https://reqnroll.net
BSD 3-Clause "New" or "Revised" License
302 stars 33 forks source link

Autofac without `BeforeTestRun` hook does not run `GlobalDependencies` #127

Closed michielboekhoff closed 1 month ago

michielboekhoff commented 1 month ago

Reqnroll Version

1.0.1

Which test runner are you using?

xUnit

Test Runner Version Number

2.7.0

.NET Implementation

.NET 8.0

Test Execution Method

Other – PLEASE SPECIFY

Content of reqnroll.json configuration file

{
}

Issue Description

Hey there! First of all, thanks so much for this project. I'm really happy that it's been picked up, and I'm really happy to see how active this repo has been recently.

Since I've started using the Autofac plugin, I've been seeing mildly strange behaviour with the [GlobalDependencies] attribute. I've been writing integration tests using Testcontainers, and I want to set up a single container to share across all features. To this end, I've got a fairly complex GlobalDependencies class:

GlobalDependencies.cs

public class GlobalDependencies
{
    [GlobalDependencies]
    public static void RegisterGlobalDependencies(ContainerBuilder builder)
    {
        builder
            .Register(_ =>
            {
                var container = new WireMockContainerBuilder()
                    .WithAutoRemove(true)
                    .WithCleanUp(true)
                    .Build();

                // This is not ideal, but perhaps we can do this in the `BeforeTestRun`
                container.StartAsync().Wait();

                return container;
            })
            .Named<WireMockContainer>("my-downstream-service-1")
            .SingleInstance();

        builder.Register((c, p) =>
            {
                var image = ServiceImage();
                var regulations = p.Named<WireMockContainer>("my-downstream-service-1");
                var container = new TestContainerBuilder()
                    .WithImage(image)
                    .WithAutoRemove(true)
                    .WithCleanUp(true)
                    .WithPortBinding(80, assignRandomHostPort: true)
                    .WithWaitStrategy(Wait.ForUnixContainer().UntilHttpRequestIsSucceeded(r => r.ForPort(80)))
                    .Build();

                container.StartAsync().Wait();

                return container;
            })
            .As<IContainer>()
            .AutoActivate();

        builder.Register((_, p) => p.Named<WireMockContainer>("my-downstream-service-1").CreateWireMockAdminClient())
            .Named<IWireMockAdminApi>("my-downstream-service-1-wiremock-admin");
    }

    private static IFutureDockerImage ServiceImage()
    {
        var image = /* some image building... */;
        image.CreateAsync().Wait();
        return image;
    }
}

ScenarioDependencies.cs

public class ScenarioDependencies
{

  [ScenarioDependencies]
  public static void CreateContainerBuilder(ContainerBuilder containerBuilder)
  {
    containerBuilder.AddReqnrollBindings<ScenarioDependencies>();
  }

}

The code annotated with GlobalDependencies runs only if I have a [BeforeTestRun] hook.

After some investigation, I believe it's because of an issue in the BindingRegistryExtensions class. It appears that, at the time of the global container registration, the IBindingRegistry is not ready yet (understandable, as it can't map the step definitions). Without any [BeforeTestRun] hooks, GetBindingTypes returns an empty IEnumerable and GetBindingAssemblies returns an empty IEnumerable too. This ultimately means that GetMethod in ContainerBuilderFinder.cs returns null which causes this condition to trigger:

        protected virtual Func<ContainerBuilder, ContainerBuilder> FindCreateScenarioContainerBuilder(Type attributeType, Type returnType, Func<ContainerBuilder, MethodInfo, ContainerBuilder> invoke)
        {
            var method = GetMethod(attributeType, returnType);

            return method == null
                ? null
                : containerBuilder => invoke(containerBuilder, method);
        }

(e.g., the code thinks there is no [GlobalDependencies]-annotated method!)

Steps to Reproduce

Create a project which uses [GlobalDependencies], uses the Autofac plugin but has no hooks. Add an empty [BeforeTestRun] hook like this:

[Binding]
public class BeforeTestRun
{
    [BeforeTestRun]
    public static void Foo()
    {
    }
}

to make the issue disappear.

I can also set up a repro project if desired!

Link to Repro Project

No response

michielboekhoff commented 1 month ago

Okay, so, digging a bit deeper still - it's because we can't access things from the global context inside of our step definitions. In my use-case, have you got any advice on how to achieve this?

gasparnagy commented 1 month ago

@michielboekhoff You can let the ITestRunContext injected, that is the global context. It has a TestRunContainer property, if you need the DI container.

gasparnagy commented 1 month ago

@michielboekhoff I have also run into a similar issue while trying to fix #56, but not exactly yours and I could not reproduce your case.

What I have found was that before processing the [GlobalDependencies] method, the plugin calls the binding registry building here so the binding registry should be ready by that time. Is it possible that the [GlobalDependencies] method in your case was not in the Reqnroll project but in another project?

gasparnagy commented 1 month ago

@michielboekhoff Ignore my last comment. I was able to reproduce the problem. It seems that any hook fixes the problem, not only [BeforeTestRun]... Strange... But at the end my fix to #56 also fixes this one, so I'm not sure I want to fully understand how it happened.

gasparnagy commented 1 month ago

OK. I understand it now why it was wrong. Basically the IBindingRegistry was half finished at the time the [GlobalDependencies] method was searched for. The step definitions were not processed yet, therefore they cannot report their assemblies. The hooks are immediately processed, so they reported the assembly. This is why the hook was needed. It is fixed now by #139