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

xUnit ICollectionFixture<TFixture> injects new instance of TFixture into steps instead of pre-initialized one. #1430

Open
6 of 25 tasks
sphinxy opened this issue Mar 11, 2019 · 8 comments

Comments

@sphinxy
Copy link
Contributor

sphinxy commented Mar 11, 2019

SpecFlow Version:

  • 3.0 (3.0.169-beta)
  • 2.4
  • 2.3
  • 2.2
  • 2.1
  • 2.0
  • 1.9

Used Test Runner

  • SpecFlow+Runner
  • MSTest
  • NUnit
  • Xunit

Version number: 3.0.169 - beta

Visual Studio Version

  • VS 2017 15.9.7
  • VS 2015
  • VS 2013

Are the latest Visual Studio updates installed?

  • Yes
  • No

.NET Framework:

  • >= .NET 4.5
  • before .NET 4.5
  • .NET Core 2.0
  • .NET Core 2.1
  • .NET Core 2.2
  • .NET Core 3.0

Test Execution Method:

  • Visual Studio Test Explorer
  • TFS/VSTS/Azure DevOps – Task – PLEASE SPECIFY THE NAME OF THE TASK
  • Command line – PLEASE SPECIFY THE FULL COMMAND LINE

<SpecFlow> Section in app.config

{
  "generator": {
    "allowDebugGeneratedFiles": true
  }
}

Repro Project

https://github.com/sphinxy/SpecFlow3Core.XunitPlayground

Issue Description

Native Xunit runner supports ICollectionFixture, where TFixtureis instantiated once per collection and later can be injected to tests.

If we use this approach via specflow xUnit runner with adding tag @xunit:collection(SampleCollection) to support collection and injection TFixture, TFixture is instantiated twice - once for before collection run as expected (by xunit.execution.dotnet.dll!Xunit.Sdk.XunitTestCollectionRunner.CreateCollectionFixture.AnonymousMethod) and then another instance once again (by BoDi.dll!BoDi.ObjectContainer.CreateObject() )
for injecting into specflow steps.

BoDi should reuse existing instance

Steps to Reproduce

Use Xunit 2 runner. Create sample specflow feature and steps. Create Xunit ICollectionFixture and force specflow to use it via @xunit:collection tag in feature file.
Inject TFixture into steps constructor

Check by debug that TFixture is created twice.

Extra: implement IAsyncLifetime in TFixture and check that second instance retrieved in steps doesn't calls InitializeAsync method but first instance for collection do.

@sphinxy sphinxy changed the title xUnit ICollectionFixture<TFixture> inject new instance of TFixture into steps instead of pre-initialized one. xUnit ICollectionFixture<TFixture> injects new instance of TFixture into steps instead of pre-initialized one. Mar 11, 2019
@sphinxy
Copy link
Contributor Author

sphinxy commented Mar 11, 2019

I've check current implementation and it seems that we should register this TFixture in xUnit generated code like we already do for ITestOutputHelper:
testRunner.ScenarioContext.ScenarioContainer.RegisterInstanceAs<TFixture>(_tFixture);

A problem that namespace of this TFixture is unknown, it's somewhere in custom user code, so I don't know how to use it.

All info that we have in generated code is [Xunit.Collection("SampleCollection")], it's not even a type but just a collection name. Native xUnit runner use reflection to get collection class by that name and then to find TFixture class from collection class.

Just ideas:

  1. Use same reflection approach as native xUnit runner to find TFixture class and register it in Bodi.
  2. Maybe try to use power of partial class and somehow get TFixture instance.
  3. Extend collection tag and provide fully qualified name of TFixture, use it to generate code.

@SabotageAndi
Copy link
Contributor

How is IAsyncLifetime used in XUnit? I don't know this interface.

@sphinxy
Copy link
Contributor Author

sphinxy commented Mar 14, 2019

How is IAsyncLifetime used in XUnit? I don't know this interface.

"A new IAsyncLifetime interface is available, which can be placed onto test classes, class fixtures, and collection fixtures. This allows asynchonous initialization (InitializeAsync) and cleanup (DisposeAsync)."

Current behaviour: with adding of @xunit:collection(SampleCollection), xunit recognize collection, then recognize collection TFixture and if IAsyncLifetime added, run InitializeAsync for this TFixture.

Here, https://github.com/xunit/xunit/blob/22fc5443/src/xunit.execution/Sdk/Frameworks/Runners/XunitTestCollectionRunner.cs#L118

Basically, it is for the same purpose as [BeforeTests] tag in specflow/cucumber.

@sai-nagarjuna-t
Copy link

Thanks for raising this issue. Any idea when this will be fixed or any workaround in place?
Thanks.
@SabotageAndi @sphinxy

@sphinxy
Copy link
Contributor Author

sphinxy commented Apr 24, 2019

Thanks for raising this issue. Any idea when this will be fixed or any workaround in place?
I'll try to implement the fix on this weekend, but chances for success are not so high, because it might require heavy usage of reflection or introduce very specific tag for specifying TFixture class name too, both approaches are not good.

@Eshva
Copy link

Eshva commented Dec 23, 2019

Namaste!

  1. Use same reflection approach as native xUnit runner to find TFixture class and register it in Bodi.
  2. Maybe try to use power of partial class and somehow get TFixture instance.
  3. Extend collection tag and provide fully qualified name of TFixture, use it to generate code.

I guess the option №3 is better because:

  1. This tag @xunit:collection is a hack already. It's very hard to find information about it at least.
  2. Probably value of name parameter of CollectionAttribute must not be a class name. It could be any string that shared among all classes: test collection (CollectionDefinitionAttribute in this case), test fixture and test classes.

In case of the option №1 we should trust to the user that the name value of @xunit:collection is the class name. If we will require full class name of the collection fixture and document it, it would be at least fair.
I guess we should add one more tag @xunit:collectionFixtureFullClassName (or something like this). We need it because test collections have 2 purposes:

  1. Test run parallelism boundaries.
  2. A way to share execution context among different test classes.
    The tag xunit:collection should serve the first purpose and allow any string (currently it doesn't allow strings with spaces for instance). Additionally it should specify the test category much like it does now but not like this:
[Xunit.TraitAttribute("Category", "xunit:collection(CallectionName)")]

but like this:

[Xunit.TraitAttribute("Category", "Some test collection name possibly with spaces.")]

The new tag @xunit:collectionFixtureFullClassName should serve the second purpose: set the full name of the test collection fixture.

Or even better add @xunit:collectionFullClassName tag that should be mutually exclusive. If we know the Collection class name, we can get it's name from CollectionDefinitionAttribute on that collection class and the fixture class name from the collection class definition.

Any other suggestions?

@rdasan
Copy link

rdasan commented Feb 16, 2021

Any plans on addressing this issue?

@bonek
Copy link

bonek commented Aug 26, 2022

You can write RuntimePlugin and GeneratorPlugin yourself overrride XUnit2TestGeneratorProvider. I wrote own exacly like the option №3. But I had to use reflection to invoke "FeatureStart Event" after registering in featureContainer.

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

No branches or pull requests

6 participants