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

MSBuild always regenerates code-behind .cs files #1822

Open
9 of 33 tasks
flcdrg opened this issue Dec 17, 2019 · 17 comments
Open
9 of 33 tasks

MSBuild always regenerates code-behind .cs files #1822

flcdrg opened this issue Dec 17, 2019 · 17 comments

Comments

@flcdrg
Copy link
Contributor

flcdrg commented Dec 17, 2019

SpecFlow Version:

  • 3.1
  • 3.0
  • 2.4
  • 2.3
  • 2.2
  • 2.1
  • 2.0
  • 1.9

Used Test Runner

  • SpecFlow+Runner
  • MSTest
  • NUnit
  • Xunit

Version number: 3.1.67

Project Format of the SpecFlow project

  • Classic project format using packages.config
  • Classic project format using <PackageReference> tags
  • Sdk-style project format

.feature.cs files are generated using

  • SpecFlow.Tools.MsBuild.Generation NuGet package
  • SpecFlowSingleFileGenerator custom tool

Visual Studio Version

  • VS 2019
  • VS 2017
  • VS 2015

Enable SpecFlowSingleFileGenerator Custom Tool option in Visual Studio extension settings

  • Enabled
  • Disabled

Are the latest Visual Studio updates installed?

  • Yes
  • No, I use Visual Studio version <Major>.<Minor>.<Patch>

.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

Issue Description

It appears that the current implementation of the UpdateFeatureFilesInProject target in SpecFlow.Tools.MsBuild.Generation.targets means that feature.cs files will be regenerated every time the project builds, regardless of whether any source .feature file has changed.

Is there a reason why incremental building isn't supported?

(If there isn't, then I'd be up for having a go at adding it)

@flcdrg
Copy link
Contributor Author

flcdrg commented Dec 18, 2019

I found that @david1995 removed this back in 70a9b36.

Was there a problem with the inputs/outputs?

@SabotageAndi
Copy link
Contributor

The problem was, that Inputs/Outputs checks only for existence of the files and not the content. That makes us problems, as there are some configs that adjust the generated code.

We have an optimization in the code, that we only write to the filesystem when the content changed. It is here: https://github.com/techtalk/SpecFlow/blob/master/SpecFlow.Tools.MsBuild.Generation/CodeBehindWriter.cs#L17

@flcdrg
Copy link
Contributor Author

flcdrg commented Dec 19, 2019

Inputs / Outputs also compares file modification times, so that would be the equivalent of when the content changes.

I suspect a problem with the linked code is it still does the processing so there's still a performance impact, even if it doesn't write the changes.

@pergardebrink
Copy link
Contributor

pergardebrink commented Aug 15, 2020

What configuration values is it dependant on? If using Inputs/Outputs is a problem because of configuration values, you could make a hash of all those values and write the hash to a cache file that you use as additional inputs to the target to determine if UpdateFeatureFilesInProject should incrementally built. Then it will also be triggered when configuration changes (as well as feature files).

This is how the assemblyinfo is generated only if the version and other properties are changed: https://github.com/dotnet/sdk/blob/18ee4eac8b3abe6d554d2e0c39d8952da0f23ce5/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.GenerateAssemblyInfo.targets#L134

Used here as Inputs: https://github.com/dotnet/sdk/blob/18ee4eac8b3abe6d554d2e0c39d8952da0f23ce5/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.GenerateAssemblyInfo.targets#L152

@flcdrg
Copy link
Contributor Author

flcdrg commented Aug 15, 2020

Would definitely like to see this fixed. Not allowing MSBuild to figure out that there's nothing to do means builds are much more inefficient.

@reegeek
Copy link

reegeek commented Apr 2, 2021

Hi,
What is the status of this issue ?
Is there a temporary solution ?

Regards

@pergardebrink
Copy link
Contributor

Hi,
What is the status of this issue ?
Is there a temporary solution ?

Regards

I think I fixed this in #2094 but it was a long time ago, so not sure it was the same issue? My fix didn't write the files unless it had to..

Are you seeing this issue still?

@SabotageAndi
Copy link
Contributor

@reegeek Nothing changed and no there is no temporary solution.

@pergardebrink If I remember correctly, it is about that the MSBuild task is called at all.

@SabotageAndi
Copy link
Contributor

But as SpecFlow is Open Source and you are annoyed with this issue, please submit a PR to fix it. We are only a small team and can't do everything. SpecFlow is dependent on the contributions of its users.

Next week we are having again an open-source iteration (https://specflow.org/blog/our-second-open-source-iteration-starts-next-week/). The whole team will be available to mentor future contributors and help them with their change to SpecFlow.

@flcdrg
Copy link
Contributor Author

flcdrg commented Apr 5, 2021

If someone wanted to test this locally, find your local copy of SpecFlow.Tools.MsBuild.Generation.targets and revert the change made in the commit mentioned above and see if it behaves correctly. If it does, then consider raising a PR.

I've moved onto different work that isn't using SpecFlow at the moment, otherwise I'd do it myself.

@reegeek
Copy link

reegeek commented Apr 6, 2021

I will made the test and maybe raising a PR.

@SabotageAndi
Copy link
Contributor

Thanks, @reegeek for the PR, but it is only adding the Inputs/Outputs to the target, which as already written is not enough.

The generated code is not only dependent on the input files, but also on:

When I would change one of these, the target wouldn't be called at all and the user is stuck with the old generated code.

So something like @pergardebrink suggested would be necessary to handle this cases.

@reegeek
Copy link

reegeek commented Apr 6, 2021

Ok, if I understand well:
If unit test framework changes, we must regenerate output files.
How to know which config values should regenerate files ?

@SabotageAndi
Copy link
Contributor

The generator section has two possible properties and both of them (allowDebugGeneratedFiles, allowRowTests) adjust the generated code.

@reegeek
Copy link

reegeek commented Apr 6, 2021

ok.
If we add in "app.config" file and "specflow.json" file Inputs, I think it is solving the second point.

@SabotageAndi
Copy link
Contributor

Yes, that could be enough.

@reegeek
Copy link

reegeek commented Apr 12, 2021

Hi,
I add configuration files and plugins files in inputs.

  <ItemGroup>
    <!-- features files -->
    <UpdateFeatureFilesInProjectInputs Include="@(SpecFlowFeatureFiles)"/>
    <!-- configuration files -->
    <UpdateFeatureFilesInProjectInputs Include="**\specflow.json"/>
    <UpdateFeatureFilesInProjectInputs Include="**\app.config" />
    <!-- plugin files -->
    <UpdateFeatureFilesInProjectInputs Include="@(SpecFlowGeneratorPlugins)"/>
  </ItemGroup>

@SabotageAndi I think it is enough.

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

Successfully merging a pull request may close this issue.

4 participants