Support for empty RootNamespace broken in 3.1

idg10 commented 4 years ago

SpecFlow Version:

[x] 3.1
[ ] 3.0
[ ] 2.4 N/A
[ ] 2.3 N/A
[ ] 2.2 N/A
[ ] 2.1 N/A
[ ] 2.0 N/A
[ ] 1.9 N/A

Used Test Runner

[ ] SpecFlow+Runner
[ ] MSTest
[x] NUnit
[ ] Xunit

Version number: SpecFlow.NUnit.Runners Version 3.1.74

Project Format of the SpecFlow project

[ ] Classic project format using packages.config
[ ] Classic project format using <PackageReference> tags
[x] Sdk-style project format

.feature.cs files are generated using

[x] SpecFlow.Tools.MsBuild.Generation NuGet package
[ ] SpecFlowSingleFileGenerator custom tool

Visual Studio Version

[x] VS 2019
[ ] VS 2017
[ ] VS 2015

`Enable SpecFlowSingleFileGenerator Custom Tool` option in Visual Studio extension settings

[ ] Enabled
[x] Disabled

Are the latest Visual Studio updates installed?

[x] Yes
[ ] No, I use Visual Studio version <Major>.<Minor>.<Patch>

.NET Framework:

[ ] >= .NET 4.5 (not tried)
[ ] before .NET 4.5 (not tried)
[ ] .NET Core 2.0 (not tried)
[x] .NET Core 2.1
[x] .NET Core 2.2
[x] .NET Core 3.0
[x] .NET Core 3.1

Test Execution Method:

[x] Visual Studio Test Explorer
[ ] TFS/VSTS/Azure DevOps – Task
[x] Command line – dotnet test

<SpecFlow> Section in app.config or content of specflow.json

No specflow.json required to cause this bug to show. However, I added one just to make sure, and it didn't change anything:

{
    "bindingCulture":
    {
        "language" :"en-us"
    },
    "language":
    {
        "feature": "en-us"
    }
}

Repro Project

https://github.com/idg10/SpecFlowNamespaceBug

Issue Description

Last year, I submitted https://github.com/techtalk/SpecFlow/pull/1606 which made it possible for a SpecFlow project to specify an empty <RootNamespace> in its csproj file. This is useful because it enables projects' folder structure to exactly reflect the namespace structure. In situations where there is more than one root namespace, this is the only good way to arrange your files if you want VS's code generation to work with you.

For example, in a project I partly maintain, a library mostly puts its code into a namespace Corvus.Monitoring, but in order to enable discoverability during DI initialization, it also puts certain types in Microsoft.Extensions.DependencyInjection. You can see this reflected in the folder structure at https://github.com/corvus-dotnet/Corvus.Monitoring/tree/master/Solutions/Corvus.Monitoring.Instrumentation.Abstractions

In projects that need to do this, there is no common namespace prefix, so it makes sense to have an empty <RootNamespace>. This is why I submitted https://github.com/techtalk/SpecFlow/pull/1606 last year - I wanted my test projects to have a consistent layout with my projects under test. SpecFlow 3.0 included this change, and we've successfully been using it in projects with empty root namespaces ever since.

Unfortunately, SpecFlow 3.1 has regressed: it does not work with projects that work this way.

Steps to Reproduce

If you open the repro project at https://github.com/idg10/SpecFlowNamespaceBug you'll find that the TestWithSpecFlow30 project works just fine—it is built against SpecFlow 3.0, which supports empty root namespaces. But TestWithSpecflow31 fails to build, because SpecFlow 3.1 broke this support. It fails with this build error:

3>C:\Users\ian\.nuget\packages\specflow.nunit\3.1.74\build\SpecFlow.NUnit.targets(34,5): error MSB4044: The "ReplaceTokenInFileTask" task was not given a value for the required parameter "TextToReplaceWith".

The failure appears to be in the ReplaceTokenInFileTask MSBuild custom task. Apparently this was added in https://github.com/techtalk/SpecFlow/pull/1774 and the goal seems to have been that when a class previously called NUnitAssemblyHooks would have been added, that classes name would now incorporate a lightly-mangled version of the project root namespace. E.g., it might be called MyOrg_MyProject_NUnitAssemblyHooks. And the problem is that this change was made on the assumption that there was a non-empty root namespace, an assumption that does not always hold.

SabotageAndi commented 4 years ago

Oh sorry, I completely forgot that usecase with an empty RootNamespace. Your PR looks good. Thanks for fixing the problem. I will merge it and release new packages when the build pipeline is finished. So some when tomorrow new packages should be available with the fix.

github-actions[bot] commented 3 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

SpecFlowOSS / SpecFlow