SpecFlow icon indicating copy to clipboard operation
SpecFlow copied to clipboard

Make specflow config file name (specflow.json) configurable

Open epresi opened this issue 2 years ago • 7 comments

Possible solution for #2546

Example usage:

<PropertyGroup>
    <TargetFramework>net6.0</TargetFramework>
    <Nullable>enable</Nullable>
    <ImplicitUsings>enable</ImplicitUsings>
    <SpecFlowConfigFileName>specflow.dev.json</SpecFlowConfigFileName>
</PropertyGroup>

but it supports MsBuild properties as well:

<SpecFlowConfigFileName>$(AssemblyName).specflow.json</SpecFlowConfigFileName>

or whatever:

<SpecFlowConfigFileName>apple.json</SpecFlowConfigFileName>

If the SpecFlowConfigFileName is not specified, then the default value is used, which is specflow.json.

Sample test output/log:

  Standard Output: 
-> Loading plugin D:\Git\SpecFlowProject1\SpecFlowProject1\bin\Debug\net6.0\TechTalk.SpecFlow.MSTest.SpecFlowPlugin.dll
-> Loading plugin D:\Git\SpecFlowProject1\SpecFlowProject1\bin\Debug\net6.0\SpecFlowProject1.dll
-> Using apple.json

It's just adjusting the file name, if the content is not a valid specflow.json it will behave as before.

What is missing: adding some tests for full coverage, docs, changelog. Just opened the PR so that I'll get a review if it is worth the time to finish this PR.

Types of changes

  • [ ] Bug fix (non-breaking change which fixes an issue).
  • [x] New feature (non-breaking change which adds functionality).
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected).
  • [ ] Performance improvement
  • [ ] Refactoring (so no functional change)
  • [ ] Other (docs, build config, etc)

Checklist:

  • [ ] I've added tests for my code. (most of the time mandatory)
  • [ ] I have added an entry to the changelog. (mandatory)
  • [ ] My change requires a change to the documentation.
  • [ ] I have updated the documentation accordingly.

epresi avatar Jul 28 '22 15:07 epresi

What I realized in the meanwhile is that during building a SpecFlow project, the MsBuild.Generation writes to the output the used config: ConfigurationLoader Now the name of the config file is not the one I added, because of the GeneratorContainerBuilder's new container. SpecFlowConfigFileNameProvider is registered in the parentObjectContainer, not in the new container. Is there an elegant way to check if it is registered in the parent container? Because .IsRegistered<SpecFlowConfigFileNameProvider>() returns false on the child container (but it is possible to resolve it, I guess it resolves from the parent?!), therefore my code registers SpecFlowConfigFileNameProvider in the child container with the default value, which will be resolved later.

epresi avatar Jul 28 '22 15:07 epresi

The registration issue we can figure out. But I think I am missing something. As I understood it, you convert the MSBuild property to an assembly attribute, which contains the configuration file name. But where do you read this value? At compile time you get it as MSBuild task parameter, but I don't see anything that uses Reflection at runtime to get the value. Am I blind? Or still to tired?

SabotageAndi avatar Jul 29 '22 07:07 SabotageAndi

Yes, compile time it is a parameter, at runtime it is an assembly attribute. Reading and setting is done in the ContainerBuilder

epresi avatar Jul 29 '22 08:07 epresi

Aaahh!

SabotageAndi avatar Jul 29 '22 08:07 SabotageAndi

Is the issue in the Generator or at Runtime?

SabotageAndi avatar Jul 29 '22 12:07 SabotageAndi

In Generator, Runtime is a bit different, there is no parent container, so it's registered in the right one.

epresi avatar Jul 29 '22 14:07 epresi

@SabotageAndi @lanfeust69 I have discussed this with @epresi, and I don't think it would be a good idea to make the config file configurable, because of the complications with the tooling (IDE extensions, etc.). I think we could find a better alternative for the original problem, without making this change in the SpecFlow core. I will share my ideas at the original issue.

gasparnagy avatar Aug 03 '22 09:08 gasparnagy

Closing it based on the comment earlier.

gasparnagy avatar Oct 20 '22 10:10 gasparnagy