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

Looking for clarification on Before/AfterFeature guarantees when running method-level test parallelism #345

Open
DrEsteban opened this issue Dec 8, 2024 · 5 comments
Labels
documentation Improvements or additions to documentation

Comments

@DrEsteban
Copy link

DrEsteban commented Dec 8, 2024

Related Documentation Page

https://docs.reqnroll.net/latest/execution/parallel-execution.html

Type of the problem

Missing information

Problem Description

The Parallel Execution page says:

  • As a general guideline, we do not suggest using [BeforeFeature] and [AfterFeature] hooks and the FeatureContext when running the tests parallel, because in that case it is not guaranteed that these hooks will be executed only once and there will be only one instance of FeatureContext per feature. The lifetime of the FeatureContext (that starts and finishes by invoking the [BeforeFeature] and [AfterFeature] hooks) is the consecutive execution of scenarios of a feature on the same parallel execution worker thread. In case of running the scenarios parallel, the scenarios of a feature might be distributed to multiple workers and therefore might have their own dedicated FeatureContext. Because of this behavior the FeatureContext is never shared between parallel threads so it does not have to be handled in a thread-safe way. If you wish to have a singleton FeatureContext and [BeforeFeature] and [AfterFeature] hook execution, scenarios in a feature must be executed on the same thread.

Despite this well-reasoned warning, I still have a desire to use [BeforeFeature] and [AfterFeature] in my test project 😄

I just want to clarify: As long as I'm not expecting singleton-like behavior for each Feature and FeatureContext, am I guaranteed that every scenario will have the [BeforeFeature] and [AfterFeature] hooks called as expected? Including access to the dependencies registered in those methods?

For example, would the following things be valid?

  1. Consistent context objects per Feature-level DI container
  2. Consistent dictionary values per FeatureContext
  3. Manually ensured singleton behavior per-Feature

1. Consistent context objects per Feature-level DI container

[BeforeFeature]
public static void RegisterResourceTracker(IObjectContainer container)
{
  container.RegisterInstanceAs(new ResourceTracker());
}

// Scenario creates resources and stores them in the ResourceTracker

[AfterFeature]
public static async Task CleanupResourcesAsync(IObjectContainer container)
{
  var tracker = container.ResolveRequired<ResourceTracker>();
  foreach (var resource in tracker.CreatedResources)
  {
    await resource.DeleteAsync();
  }
}

☝🏻 I realize that multiple ResourceTrackers might be created for the same Feature, due to the parallelization, but will it at least remain a consistent reference for the entire execution of the Scenario?

2. Consistent dictionary values per FeatureContext

Similar to the first example - since all *Context objects are also dictionaries, will anything set in in a [BeforeFeature] remain consistent for the duration of a Scenario?

[BeforeFeature]
public static void RegisterContextInformation(FeatureContext context)
{
  context.Set(DateTimeOffset.Now, "StartedTime");
}

[AfterFeature]
public static void LogDone(FeatureContext context, IReqnrollOutputHelper output)
{
  output.WriteLine($"Duration: {DateTimeOffset.Now - context["StartedTime"]}");
}

☝🏻 I recognize this example doesn't make sense haha, but is it technically possible?

3. Manually ensured singleton behavior per-Feature

private static readonly ConcurrentDictionary<string, MyDependency> PerFeatureDependencies = new();

[BeforeFeature]
public static void RegisterPerFeatureDependency(FeatureContext context, IObjectContainer container)
{
  var dependency = PerFeatureDependencies.GetOrAdd(featureContext.FeatureInfo.Title, () => new MyDependency());
  container.RegisterInstanceAs(dependency);
}

☝🏻 Would that effectively make dependencies behave as if parallelization was disabled or at the Feature level?


Thanks for this awesome project, and thanks in advance for your response!

Please let me know if this is better as a Discussion rather than Issue, but I figured it might be a good idea to update the documentation a bit to clarify what users should expect.

@DrEsteban DrEsteban added the documentation Improvements or additions to documentation label Dec 8, 2024
@obligaron
Copy link
Contributor

  1. Yes, it will be consistent. The same FeatureContainer is used for the Before and After Feature Hooks.
  2. Yes, it will be consistent. The example is possible 😉.
  3. Yes, you can do it this way. But you have to consider if you need some kind of cleanup. then you have to do it manually. Regarding cleanup: any instance registered in an ObjectContainer that implements IDisposable will be disposed of when the ObjectContainer is disposed of. So in your example, if MyDependency implements IDisposable, it will be disposed of when the first feature context is disposed of. This is likely not desired.

Hope this helps a bit. 🙂

@gasparnagy
Copy link
Contributor

@DrEsteban I have been struggling quite much to describe the behavior in an understandable way in the docs, so feel free to send a PR for updating it based on the info you learn here. Probably others might have similar questions too.

@DrEsteban
Copy link
Author

@gasparnagy How do you feel about me adding my example from 3. above in the docs? As a way to work around the behavior you're warning about, and to help explain what users can expect? (Giving an example of a work around might help users understand the problem itself.)

Or would you prefer not to be that prescriptive in the official docs?

@DrEsteban
Copy link
Author

Hmmm thinking about it again, perhaps it's not the right example. Because it comes with the caveats @obligaron very smartly pointed out, e.g. IDisposable considerations.

I think I see what you mean 😄 It's a topic that's very hard to describe succinctly! Especially for junior .NET developers. I can see now why you decided to just recommend against it, otherwise the docs could get quite lengthy calling out all the edge cases and caveats 😆

@DrEsteban
Copy link
Author

I took a stab at it! #364

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

No branches or pull requests

3 participants