Draft: Add Roslyn source generator

Code-Grump commented 4 days ago

🤔 What's changed?

Adds new Roslyn-based source generator to convert feature files into executable tests.

⚡️ What's your motivation?

🏷️ What kind of change is this?

:zap: New feature (non-breaking change which adds new behaviour)

♻️ Anything particular you want feedback on?

The code structures for the modelling of features and scenarios
The models for test fixtures and methods
The mechanisms for rendering source code
The tests to validate surface-level code generation

📋 Checklist:

[ ] I've changed the behaviour of the code
- [ ] I have added/updated tests to cover my changes.
[ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
[ ] Users should know about my change
- [ ] I have added an entry to the "[vNext]" section of the CHANGELOG, linking to this pull request & included my GitHub handle to the release contributors list.

This text was originally taken from the template of the Cucumber project, then edited by hand. You can modify the template here.

Code-Grump commented 4 days ago

My current focus is to get all test scenarios in the System tests to pass for MSTest, but I'm not able to decipher the test-failure for scenario. The output is both massive and cryptic, and I would appreciate help translating the failure into something actionable.

But I'm also slightly thwarted by the build here failing for the Analyser package as it's not your standard NuGet package.

obligaron commented 4 days ago

Do you mean the Handles_different_scenario_and_scenario_outline_outcomes test?

This one fails, because the logic in Roslyn Source Generator is missing a special case for ignored examples.

In the old generator the special logic is in MsTestV2GeneratorProvider:

// MsTest doesn't support to ignore a specific test case / DataRow
if (isIgnored)
{
    return;
}

If I add a check for the ignore tag to the GenerateTestMethodAttributes method in the roslyn source generator, the tests runs fine:

var moreData = values.Skip(1).ToList();

if (set.Tags.Any(x => x.Equals("ignore", StringComparison.InvariantCultureIgnoreCase)))
    continue;

moreData.Add(set.Tags);

Hope that helps. 🙂

Code-Grump commented 4 days ago

Ignored examples don't get executed, so I dropped in a little config switch to enable emitting / omitting code for these examples. I made the default to omit examples with an ignore tag and all the MSTest examples pass now. Next up: fixing the NuGet packaging failure in the build pipeline.

Code-Grump commented 4 days ago

The error NU5017 is quite specific, relating to a lack of assemblies or assembly references in a package: https://learn.microsoft.com/en-us/nuget/reference/errors-and-warnings/nu5017

This is true for Roslyn packages and I don't see this error building locally, even when running an identical dotnet build command as seen in the CI pipeline. I am clueless and in need of further help.

obligaron commented 3 days ago

The CI sets two environment variables to true, if you do the same you can reproduce it locally:

set REQNROLL_TEST_PIPELINEMODE=true
set GITHUB_ACTIONS=true

Likely the second one is causing the problem, as the following is configured in Directory.Build.props:

  <PropertyGroup Condition="'$(GITHUB_ACTIONS)' == 'true'">
    <GitBranch Condition=" '$(GitBranch)' == ''">$(GITHUB_REF_NAME)</GitBranch>
    <GitCommitSha Condition=" '$(GitCommitSha)' == ''">$(GITHUB_SHA)</GitCommitSha>
    <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>
    <EmbedUntrackedSources>true</EmbedUntrackedSources>
    <IncludeSymbols>true</IncludeSymbols>
    <SymbolPackageFormat>snupkg</SymbolPackageFormat>
  </PropertyGroup>

This also creates a snupkg and this snupkg fails to generate. Unfortunately, the error message is not very good. There is also an issue to improve the error message in the NuGet Repro (https://github.com/NuGet/Home/issues/10372).

A quick fix would be to disable the inclusion of the pdb (and also the snupkg) in the FeatureTestGenerator.csproj:

    <IncludeSymbols>false</IncludeSymbols>

Code-Grump commented 3 days ago

I think disabling the symbol packing is a good move for now. Maybe even permanently. Progress for now.

reqnroll / Reqnroll