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

Removing CodeBehind files from source #1362

Open
Code-Grump opened this issue Dec 7, 2018 · 20 comments
Open

Removing CodeBehind files from source #1362

Code-Grump opened this issue Dec 7, 2018 · 20 comments

Comments

@Code-Grump
Copy link
Contributor

One of the things I really liked about the .NET Core preview was the ability to treat the tool-generated files as build artefacts, not part of the source-controlled code. I've tried setting SpecFlowCodeBehindOutputPath in my test project, but it doesn't seem to be working.

I get the warning

For codebehind file 'obj\Debug\netcoreapp2.1\Sample.feature.cs', no feature file was found.

Followed by

error CS2001: Source file 'C:\Users\Paul\Source\Repos...\Sample.feature.cs' could not be found.

I believe that removing feature.cs from the project structure is the superior position. I'd like to propose this should be the default position for SpecFlow 3, unless there are good reasons to keep this behaviour.

I'd be happy to investigate the MSBuild changes needed to support this and put together a PR, if there's an appetite.

@SabotageAndi
Copy link
Contributor

Earlier versions of SpecFlow 3, we moved the code-behind generated files into a complete different folders.
We had problems with the MSBuild targets to get it to work everytime fine.

Additionally, Microsoft fixed a bug in VS, that the navigation from Test Explorer to the feature file works again (https://developercommunity.visualstudio.com/content/problem/267390/text-explorer-specflow-tests-goes-to-the-feature-c.html). For that, they need the code-behind files and they have to find it. Having the generated file in a complete separate folder will break this again.

So we decided, that we stay with the code- behind generated files at the same folder as the feature file.

I have to check, if we still care about the SpecFlowCodeBehindOutputPath or it's only a left over in the task.

@Code-Grump
Copy link
Contributor Author

I've been using my own SpecFlow extension to generate codebehind at build time (as a workaround for no official .NET Core support). I had to put some work in to re-bind feature files to their source code due to how the project files were generated, but I haven't seen any other issues. The tests would link through to the feature file properly, despite the generated code being in the /obj/ path.

I can't seem to follow the link you provided at the moment; it looks like the server isn't taking requests right now. I'd be happy to work through any issues this might present.

@SabotageAndi
Copy link
Contributor

We saw problems, when we put the code-behind files into the /obj/ folder and switched branches. VS/MSBuild didn't always recognized the change and used old files in a build.
Didn't you encounter such issues?

@Code-Grump
Copy link
Contributor Author

Where my extension is limited to calling specflow.exe it can only regenerate all the CodeBehind files or leave them. The step runs unless MSBuild thinks all the outputs are newer than the inputs. It shouldn't be possible to be in a situation where there is an output file older than an input file, which includes files updated by source-control operations.

<Target
  Name="GenerateSpecFlowCodeBehindCsFiles"
  BeforeTargets="BeforeBuild"
  DependsOnTargets="CreateSpecFlowShadowProject"
  Inputs="@(SpecFlowFeature->'$(SpecFlowShadowProjectPath)%(Identity)');$(SpecFlowAppConfig);$(SpecFlowShadowProject)"
  Outputs="@(SpecFlowFeature->'$(SpecFlowShadowProjectPath)%(Identity).cs')">

The only case there is potentially problem is where a file is removed (by switching the branch) and the generated codebehind file still exists in the /obj/ folder. This is solved by not including the files in the compile targets; a file is only included if there is a corresponding .feature file in the project:

<ItemGroup>
  <Compile Include="@(SpecFlowFeature->'$(SpecFlowShadowProjectPath)%(Identity).cs')" />
</ItemGroup>

@Code-Grump
Copy link
Contributor Author

As this has been open for two months without further input, let's assume there's limited interest in discussing this particular topic. Can we move forward in any way?

This is a feature I would personally like to see in SpecFlow 3; getting generated files out of source control and out of the project structure is a generally good thing. If SpecFlow is moving to exclusively generate code as a build action instead of a design-time action, I think it is a cleaner solution to treat these as intermediary build files instead of source files (precisely what the \obj folder exists for). It makes the build process a lot easier to reason about when you aren't adding files to a directory structure during a build which will influence the next build.

@SabotageAndi
Copy link
Contributor

With SpecFlow 3 we are moving nearly exclusive to generate the code-behind files during build. Nearly only, because we need to be for at least some time backward compatible.

I see no difference if we put the files under obj or any other folder. It will influence the next build in all folders. The Globbing in sdk-style projects works this way.
The obj is only a known location. And as written before, VS handles branch changes not very well without restart. So it caches files in the obj folder that aren't valid anymore. @DanielTheCoder did some experiments with that.
No idea if it is better in VS2019.

@Code-Grump
Copy link
Contributor Author

Code-Grump commented Feb 14, 2019

The difference is that when you put any files underneath the project root (but not in excluded locations like /bin and /obj), the default include mechanisms will find them on the next build unless you go to lengths to explicitly exclude them. When you put generated files into a project structure you can't ignore them from source control: they have to be tracked items to handle when you switch branches or you will have orphaned generated files appearing in the project.

If you have seen issues when experimenting with using the /obj folder and switching branches, it's probably because a glob has been used to include all the generated files on the assumption that everything in this location is valid. When you switch branches all the previous artefacts are still there until you do a clean operation. To include intermediary/generated files correctly, they have to be specified based on the inputs into the build, e.g.:

<ItemGroup>
  <Compile Include="@(SpecFlowFeature->'obj/%(Identity).cs')" />
</ItemGroup>

You can do this because you know all the inputs (the .feature files) and what should have been produced from them. Incidentally, this is also how you speed up incremental builds by specifying explicit inputs and outputs to the generation task.

Another way to look at this is it being analogous to pure functions: When the build produces a side-effect of generating new source files in the project, it's no longer free of side-effects and is much harder to explain.

@SabotageAndi
Copy link
Contributor

Yeah, I forgot that you have to manually change the .gitignore file.

From my experience, the normal globbing in the sdk-style projects include everything in the obj- Folder. We could remove all *.feature.cs files first from the Compile- ItemGroup and then add all new files to it again.

And yes, perhaps we did it wrong with our experiments. The whole MSBuild code is here: https://github.com/techtalk/SpecFlow/tree/master/SpecFlow.Tools.MsBuild.Generation/build
I am happy to review any PRs that bring the code-behind files into the obj folder.

We (@david1995 and me) have currently enough work to finish SpecFlow 3 for a final release, so at the moment we will not work on this.

@Code-Grump
Copy link
Contributor Author

I'll work on a PR for the MSBuild code.

@Code-Grump
Copy link
Contributor Author

Finally found time to spend on this, and doesn't look too hard if I take out support for generating the files within the project hierarchy. It's more challenging to support both ways of processing, due to the extra effort being spent in manipulating the <Compile> items.

@SabotageAndi, @david1995, do you think it's worth the additional complexity to continue supporting having the generated files live in the project structure? I don't object to putting in the effort; I just want to make sure it's effort well-spent.

@SabotageAndi
Copy link
Contributor

When switching branches works without problems and the Test Explorer navigation is still working, I don't care where the feature.cs -files are.
So no problem for me if you can't configure this.

@obstar
Copy link

obstar commented Jul 3, 2019

@SabotageAndi so not included code behind files '*.feature.cs' in solution explorer its a feature of SpecFlow v3?

We notice in our projects after migration to SpecFlow v3 that newly added feature files doesn't had code behind file '*.feature.cs' (well the files are there after building the solution) but despite that everything is working fine.

@david1995
Copy link
Contributor

Hi @obstar,

When using SpecFlow 3 inside a classic-style project (not SDK-style projects), you will not be able to see the .feature.cs files in the Solution Explorer. In SDK-style projects, you are able to see them as usual under the .feature file. This is afaik a limitation of MSBuild.

Adding new .feature files or modifying existing ones does not automatically generate a .feature.cs file. Code-behind generation is only triggered by building a project because SpecFlowSingleFileGenerator is not supported any more since SpecFlow 3.

Hope this helps.

@Code-Grump
Copy link
Contributor Author

I have been sitting on a working set of MSBuild changes for this, but I've been struggling to create an automated test suite for this. Any suggestions on how to proceed?

@Code-Grump
Copy link
Contributor Author

Ideally I think we need to cover the scenario where a project has been built and then a .feature file is removed, to make sure the incremental builds don't include anything from the old build. This should prove the solution works when source control operations cause feature files to be removed.

I was thinking about creating a TechTalk.SpecFlow.Specs.MSbuild project and would test by hosting the MSBuild runtime inside the test and loading a dynamic project file. The only restriction to this approach is that we would only be able to test against one version of MSBuild - the .NET Core version being targeted by the test.

Is there a range of MSBuild support that has to be covered?

@SabotageAndi
Copy link
Contributor

The SpecFlow.Tool.MSBuild.Generation packages uses MSBuild features that are available since VS2017. I think this is MSBuild 15. For that we would need the .NET Framework version (that VS uses) and the .NET Core version (from dotnet build).

Don't forget we have a test project in the solution (https://github.com/techtalk/SpecFlow/tree/master/Tests/TechTalk.SpecFlow.MsBuildNetSdk.IntegrationTests) where you can test a lot of stuff also manually.

@obstar
Copy link

obstar commented Jul 4, 2019

@david1995 @SabotageAndi
ok so in those migration steps https://specflow.org/2019/updating-to-specflow-3/ should not be step about removing from .csproj sections like this?

<Compile Include="Feature\NAME.feature.cs">
      <AutoGen>True</AutoGen>
      <DesignTime>True</DesignTime>
      <DependentUpon>NAME.feature</DependentUpon>
</Compile>

and modyfing

    <None Include="Feature\NAME.feature">
      <LastGenOutput>NAME.feature.cs</LastGenOutput>
    </None>

into

    <None Include="Feature\NAME.feature" />

also here https://specflow.org/2019/specflow-3-is-here/
should be info about it, that code behind files are generated during build and are not added anymore when user add new feature file to solution

@Code-Grump
Copy link
Contributor Author

If we want to run on just those two flavours of MSBuild (.NET Framework and .NET Core), it might be reasonable to create two test project runners and reference a shared suite of tests written as .a NET Standard library. Not something I've tried, but good in theory.

I assume you'd rather see the tests written as features too? 😃

@Code-Grump
Copy link
Contributor Author

Finally, after a great deal of not-getting-stuff-done, here's a PR for your comments:
#1691

@Code-Grump
Copy link
Contributor Author

Thanks to changes in the SpecFlow source, the new PR is a lot smaller and should be easier to discuss:
#1983

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

4 participants