SpecFlow icon indicating copy to clipboard operation
SpecFlow copied to clipboard

BeforeTestRun and AfterTestRun are static

Open godrose opened this issue 9 years ago • 17 comments

This might be the engine limitation but having these methods as static effectively denies all dependency-injection options at this point. An alternative could be adding a new set of hooks [BeforeAssemblyRun] and [AfterAssemblyRun] with support for tags. Consider the following code:

        [BeforeTestRun]
        public static void BeforeTestRun()
        {
            var setupService = _iocContainer.Resolve<ISetupService>();
            setupService.SomeSetup();
        }

Here the setup service is resolved by interface only, hiding the implementation from the consumer. This becomes a rather hard task when the method is static...

godrose avatar Sep 04 '16 05:09 godrose

@darrencauthon

  • [ ] Get the Tom Cruise "Show Me the Pull Request" loaded from your old mac to a Github template, please.

darrencauthon avatar Sep 08 '16 20:09 darrencauthon

rather than making a new attribute could you just make it so that the attribute works for static or non-static methods?

MattFenner avatar Sep 13 '16 00:09 MattFenner

We have an idea for a solution but no one has had time to implement it yet.

The solution would be to make it possible to specify parameters for these static methods and SpecFlow would pass in the proper objects from the dependency injection container. That way, we can enable parallel testing too.

dirkrombauts avatar Nov 14 '16 19:11 dirkrombauts

Is there any update on if this is going to be implemented?

Regards

alexp1980 avatar Jul 09 '18 12:07 alexp1980

@alexp1980 No update on this. Currently all of our effort is in bringing .NET Core support to SpecFlow. But PRs are always welcomed!

SabotageAndi avatar Jul 09 '18 12:07 SabotageAndi

Hey guys, do you have any update for this? will this be coming with the .NET Core support?

Flow-Joe avatar Oct 25 '18 14:10 Flow-Joe

@Tester-Joe no update, we didn't work on this.

SabotageAndi avatar Oct 25 '18 14:10 SabotageAndi

So, what ia the status on it?

greendimka avatar May 19 '19 14:05 greendimka

Nothing has changed

SabotageAndi avatar May 20 '19 07:05 SabotageAndi

@greendimka I am not aware, that anybody has this on his backlog. So if you want this feature, you will probably have to implement it by your own. If you need help, let us know in the developer gitter chat channel.

SabotageAndi avatar May 20 '19 10:05 SabotageAndi

Ok, I will have to arrange a time for it, because I have a good use case for it.

greendimka avatar May 20 '19 12:05 greendimka

So, here is the scenario, where I need BeforeTestRun to be non-static:

  • Primary goal of having BeforeTestRun as a non-static is being able to share some object within BeforeTestRun and step methods.

Gherkin lets us define objects within scenario by using tables. These is fine as long as objects are plain, for example (Room objects in a house):

Room name Floor
Kitchen 1
Bedroom 2
Bathroom 1

But as soon as you want to add an area of each floor (assume area is another object within Room object) it starts to become messy:

Room name Floor Area (X * Y)
Kitchen 1 10*5
Bedroom 2 5*5
Bathroom 1 4*5

You need a converter/retriever to read area correctly into an object. That is not the problem. The problem is that the table grows to enormous sizes when we have to deal with complex objects. I know that we should model only relevant parts of objects for the sake of testing, but sometimes relevant parts are huge. So keep the focus on the topic, not on the testing strategy :)

So I came up with an idea of moving parts (Area) of composite object (Room) to their own table within Gherkin text and to reference them later from the composite object. Areas table it looks like this:

Area-REF-ID X Y
#REF1 10 5
#REF2 5 5
#REF3 4 5

And rooms table now looks like this:

Room name Floor Area
Kitchen 1 #REF1
Bedroom 2 #REF2
Bathroom 1 #REF3

Formats of Area REF-ID and ID, used within Rooms table are not important, they may be changed and used as they are only for illustrative purposes.

Another important part of my idea is wide usage of table extensions methods from TechTalk.SpecFlow.Assist namespace, which save a lot of time and make use of converters/retrievers.

So a comparer and a retriever must be created for an Area column, which will recognize a "#REF[0-9]+" pattern and will reference an object from Areas table.

But here are the limitations (as of current /3.0.0/ SpecFlow version):

  • Areas are defined within a step, which puts all the areas into an object X, which is shared across scenario. By any means: scenario context, singleton from DI container, something else
  • Rooms table is read by using a converter
  • Converter must have an access to an X object to be able to convert #REF1 to an associated Area
  • LIMITATION! Converter is registered within a static method, which has no access to an X object due to being static.

greendimka avatar May 20 '19 20:05 greendimka

@greendimka I tried to understand your use case. But I am not sure if I have done this. Is there a reason why you don't register the Converter in the BeforeScenario Hook?

SabotageAndi avatar May 23 '19 12:05 SabotageAndi

Using Playwright with SpecFlow atm, their suggestion is to only instantiate a browser instance once per test run (because it's expensive) and favor instantiating isolated, multiple contexts per test/scenario from it.

The requested feature would perfectly fit there as well.

timmkrause avatar Oct 29 '21 06:10 timmkrause

@timmkrause Is this also the suggestion if you are doing parallel execution?

If you store the instance in the global container of SpecFlow, the Hook can still be static.

Something like that:

[BeforeTestRun]
public static void BeforeTestRunInjection(IObjectContainer globalContainer)
{
    globalContainer.RegisterInstanceAs<>(...);
}

SabotageAndi avatar Nov 03 '21 08:11 SabotageAndi

@SabotageAndi Regarding parallel test execution: Yes, that's my understanding, I haven't read/heard about exceptions in that regard. I don't know the magic behind these "contexts", but they're taking care about the isolation. When not running headless it even looks like separate browser instances. See https://youtu.be/_Jla6DyuEu4?t=1094.

Thank you for the suggestion, I didn't know about the concept of the "global container" or that it can be injected into the method.

This is how I've done it now (simplified), so I needed to capture & reuse the global container on the scenario level:

[Binding]
public class DependencyRegistrationHooks
{
    private static IObjectContainer GlobalContainer;

    [BeforeTestRun]
    public static async Task RegisterGlobalDependenciesAsync(IObjectContainer globalContainer)
    {
        var playwright = await Playwright.CreateAsync();
        var browser = await playwright.Chromium
            .LaunchAsync(
                new BrowserTypeLaunchOptions
                {
                    Headless = false
                });

        globalContainer.RegisterInstanceAs(browser);

        GlobalContainer = globalContainer;
    }

    [BeforeScenario]
    public async Task RegisterDependenciesAsync(IObjectContainer objectContainer)
    {
        await RegisterPlaywrightDependenciesAsync(objectContainer);
        // ...
    }

    private async Task RegisterPlaywrightDependenciesAsync(IObjectContainer objectContainer)
    {
        var browser = GlobalContainer.Resolve<IBrowser>();
        var context = await browser.NewContextAsync();
        var page = await context.NewPageAsync();

        objectContainer.RegisterInstanceAs(page);
    }
}

timmkrause avatar Nov 04 '21 08:11 timmkrause

@SabotageAndi Regarding parallel test execution: Yes, that's my understanding, I haven't read/heard about exceptions in that regard. I don't know the magic behind these "contexts", but they're taking care about the isolation. When not running headless it even looks like separate browser instances. See https://youtu.be/_Jla6DyuEu4?t=1094.

Thank you for the suggestion, I didn't know about the concept of the "global container" or that it can be injected into the method.

This is how I've done it now (simplified), so I needed to capture & reuse the global container on the scenario level:

[Binding]
public class DependencyRegistrationHooks
{
    private static IObjectContainer GlobalContainer;

    [BeforeTestRun]
    public static async Task RegisterGlobalDependenciesAsync(IObjectContainer globalContainer)
    {
        var playwright = await Playwright.CreateAsync();
        var browser = await playwright.Chromium
            .LaunchAsync(
                new BrowserTypeLaunchOptions
                {
                    Headless = false
                });

        globalContainer.RegisterInstanceAs(browser);

        GlobalContainer = globalContainer;
    }

    [BeforeScenario]
    public async Task RegisterDependenciesAsync(IObjectContainer objectContainer)
    {
        await RegisterPlaywrightDependenciesAsync(objectContainer);
        // ...
    }

    private async Task RegisterPlaywrightDependenciesAsync(IObjectContainer objectContainer)
    {
        var browser = GlobalContainer.Resolve<IBrowser>();
        var context = await browser.NewContextAsync();
        var page = await context.NewPageAsync();

        objectContainer.RegisterInstanceAs(page);
    }
}

There is a more correct way. For XUnit worked for me https://github.com/SpecFlowOSS/SpecFlow/issues/2580

        [BeforeTestRun(Order = 0)]
        public static void RegisterConfig(ObjectContainer objectContainer)
        {
            var configName = Environment.GetEnvironmentVariable("TEST_ENVIRONMENT");
            var config = new ConfigurationBuilder()
                .AddJsonFile(configName != null ? $"appsettings.{configName}.json" : "appsettings.json",
                    optional: false, reloadOnChange: true)
                .Build();

            objectContainer.BaseContainer.RegisterInstanceAs<IConfiguration>(config);
        }

bloodgang94 avatar Dec 06 '22 19:12 bloodgang94