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

Add Microsoft.Extensions.DependencyInjection integration plugin #94

Merged
merged 7 commits into from
May 22, 2024

Conversation

mbhoek
Copy link
Contributor

@mbhoek mbhoek commented Apr 8, 2024

As discussed in #19, this PR adds the Microsoft.Extensions.DependencyInjection integration plugin taken from solidtoken/SpecFlow.DependencyInjection@daa5409.

  • migrate tests from the original plugin (but change them according to Make contribution easier in the Reqnroll repo #72)
  • align the way this plugin works with how the Autofac plugin works (especially the different dependency scopes)
  • add documentation (usage, example, available plugins)
  • add entry to CHANGELOG

@mbhoek mbhoek added the enhancement New feature or request label Apr 8, 2024
@mbhoek mbhoek self-assigned this Apr 8, 2024
@iunal
Copy link

iunal commented May 10, 2024

Hi @mbhoek,

Thank you for your great effort.
Could you kindly give an update?

@mbhoek
Copy link
Contributor Author

mbhoek commented May 13, 2024

@gasparnagy Status update: the plugin in this PR is a direct copy of my original plugin with the namespaces adjusted. As such, it works and is ready to be merged.

However I wanted to include (new) unit tests and align the way the plugin works with how the Autofac plugin handles Global/Feature/Scenario dependencies. This has been a long standing wish of mine, because I'm not completely satisfied with how the original plugin does Feature/Scenario dependencies. It would also introduce some breaking changes in how the plugin works, which make sense to do now because we're including it in Reqnroll for the first time.

Unfortunately this turned out into a complete rewrite of the plugin, which is taking a lot longer than expected (also due to limited time). I do like where it's heading though so I would like to keep working on it. I also understand that the community is getting anxious/is waiting for this plugin, so I would like to discuss what the best next step is.

@phatcher
Copy link

How about a simple first pass that aligns with the current, and then the second version with the breaking changes?

Could have a common interface but different names so people can adopt when they have bandwidth

@gasparnagy gasparnagy added this to the v2 milestone May 14, 2024
@mbhoek
Copy link
Contributor Author

mbhoek commented May 14, 2024

I guess that's a solution and maybe the preferred one for Reqnroll's progression.

@gasparnagy Are you okay with me submitting this PR without the unit tests and aligning with Autofac's scopes (i.e. the first two items on this PRs todo list)? I could then introduce the breaking changes for the v2 milestone you already created.

@gasparnagy
Copy link
Contributor

@mbhoek sure. let's include it without the bigger changes and we will add them later

@robertcoltheart
Copy link

Just to chip in, could you consider a shorter nuget package name? For example, lots of package authors use <name>.DependencyInjection

For example:

Xunit.DependencyInjection
Npgsql.DependencyInjection
EventFlow.DependencyInjection
RavenDB.DependencyInjection

and so on.

@gasparnagy
Copy link
Contributor

@robertcoltheart The naming here is really painful, because "DependencyInjection" is such a generic name, but the correct name is too long... We have discussed about it here and decided on this long name. In our case Reqnroll.DependencyInjection is quite misleading, because Reqnroll has its own dependency injection solution. I think there is no good solution here. Let's stick to the looooong name for now. 😟

…ons-dependencyinjection-plugin

* origin/main: (21 commits)
  Fix #56 autofac ambiguous stepdef and hook required #127 issue (#139)
  Reduce target framework of Reqnroll to netstandard2.0 (#130)
  Fix StackOverflowException when using [StepArgumentTransformation] with same input and output type (#136)
  MsTest: Replace DelayedFixtureTearDown special case with ClassCleanupBehavior.EndOfClass (#128)
  Temporarily disabled tests until #132 is resolved
  Add NUnit & xUnit core tests to portability suite
  Capture ExecutionContext after every binding invoke (#126)
  small improvement in CodeDomHelper to be able to chain async calls
  fix method name sources in UnitTestFeatureGenerator
  External data plugin, support for JSON files  (#118)
  UnitTests: Check if SDK version is installed and if not ignore the test (#109)
  Fix 111 ignore attr not inherited from rule (#113)
  Update README.md (#110)
  Fix 81 - modified CucumberExpressionParameterTypeRegistry to handle multiple custom types used as cucumber expressions when those types share the same short name. (#104)
  Update index.md
  Simplify test project targets (#105)
  Make SystemTests temp folder configurable and use NUGET_PACKAGES env var to override global NuGet folder
  Fleshing out Generation System Tests (2) (#99)
  Fix for 81 - Cucumber Expression using Enums errors when two enums exist with the same short name (#100)
  Include BoDi to Reqnroll package (#91) (#95)
  ...

# Conflicts:
#	Reqnroll.sln
#	Tests/Reqnroll.PluginTests/Reqnroll.PluginTests.csproj
@gasparnagy gasparnagy marked this pull request as ready for review May 22, 2024 06:36
@gasparnagy gasparnagy changed the title [WIP] Add Microsoft.Extensions.DependencyInjection integration plugin Add Microsoft.Extensions.DependencyInjection integration plugin May 22, 2024
@gasparnagy
Copy link
Contributor

@mbhoek I have fixed the merging problems and ported the docs from https://github.com/solidtoken/SpecFlow.DependencyInjection#usage. I will merge this PR now, as I would like to make a release. We can keep improving it of course. I have made some unit tests for the Autofac plugin (see https://github.com/reqnroll/Reqnroll/blob/main/Tests/Reqnroll.PluginTests/Autofac/AutofacPluginTests.cs), maybe based on that we can also make some for this. But let's keep that for another PR.

Thx for the contribution!

@gasparnagy gasparnagy merged commit 8897591 into main May 22, 2024
4 checks passed
@gasparnagy gasparnagy deleted the add-microsoft-extensions-dependencyinjection-plugin branch May 22, 2024 07:08
@mbhoek
Copy link
Contributor Author

mbhoek commented May 22, 2024

Thanks again for taking the time to take this across the finish line @gasparnagy -- wish I could've been more helpful, but glad that the plugin is now part of Reqnroll! 👍

@bhugot
Copy link

bhugot commented May 22, 2024

Thanks to both of you, @gasparnagy any ETA for the nuget package?

@gasparnagy
Copy link
Contributor

@bhugot in a few hours...

@iunal
Copy link

iunal commented May 22, 2024

Thank you @mbhoek and @gasparnagy

gasparnagy added a commit that referenced this pull request May 22, 2024
…hread-container

* origin/main:
  Extract cucumber expression detection heuristic to an interface
  fix test artifact folder calculation
  Restructure solution folders (#141)
  cleanup CHANGELOG.md
  ignore generated file
  remove generated file
  bump version
  Add Microsoft.Extensions.DependencyInjection integration plugin (#94)
  Fix #56 autofac ambiguous stepdef and hook required #127 issue (#139)
  Reduce target framework of Reqnroll to netstandard2.0 (#130)
  Fix StackOverflowException when using [StepArgumentTransformation] with same input and output type (#136)

# Conflicts:
#	CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants