Code generator produces platform-dependent escaped line endings in string literals instead of following the line endings used in .feature files

[JefSchraag]

Reqnroll Version

3.3.1

Which test runner are you using?

NUnit

Test Runner Version Number

n/a

.NET Implementation

.NET 8.0

Test Execution Method

Visual Studio Test Explorer

Content of reqnroll.json configuration file

n/a

Issue Description

Generated *.feature.cs files should contain identical string literal content regardless of the platform where code generation occurs. The code generator should respect and follow the line ending configuration of the source *.feature files as defined in .gitattributes, using consistent escape sequences in C# string literals (\n vs \r\n) across all platforms.

Steps to Reproduce

Create a .gitattributes file:

# Explicitly declare feature files to always be normalized and converted to LF on checkout
*.feature text eol=lf

Create a feature file:

Feature: Testing line breaks

This is
a description
with line breaks

Observe the generated code on Mac:

private static global::Reqnroll.FeatureInfo featureInfo = new global::Reqnroll.FeatureInfo(new global::System.Globalization.CultureInfo("en-US"), "Features", "Testing line breaks", "This is\na description\nwith line breaks", global::Reqnroll.ProgrammingLanguage.CSharp, featureTags, InitializeCucumberMessages());

Observe the generated code on Windows:

private static global::Reqnroll.FeatureInfo featureInfo = new global::Reqnroll.FeatureInfo(new global::System.Globalization.CultureInfo("en-US"), "Features", "Testing line breaks", "This is\r\na description\r\nwith line breaks", global::Reqnroll.ProgrammingLanguage.CSharp, featureTags, InitializeCucumberMessages());

I would expect that since the line breaks are normalized to \n in the feature file (even on Windows), the generated code would be consistent across platforms.

Link to Repro Project

No response

[gasparnagy]

That's an interesting problem.

Could you please check if the behavior was the same (ie different line endings on different platforms) also in v2.4.1?

[clrudolphi]

@gasparnagy is the relevant code this section of Gerkin AstBuilder

            case RuleType.Description:
                {
                    IEnumerable<Token> lineTokens = node.GetTokens(TokenType.Other);

                    // Trim trailing empty lines
                    lineTokens = lineTokens.Reverse().SkipWhile(t => string.IsNullOrWhiteSpace(t.MatchedText)).Reverse();

                    return string.Join(Environment.NewLine, lineTokens.Select(lt => lt.MatchedText));
                }

And what role should the .editorconfig play in this?

[gasparnagy]

@gasparnagy is the relevant code this section of Gherkin AstBuilder
[...]
And what role should the .editorconfig play in this?

I don't know. We have to be very careful with this change. If my assumption is right, and this was always working like that then changing it would be a big breaking change. We need detailed planning and probably would be good to ask the Cucumber folks, how they handle this issue.

[Code-Grump]

For my curiosity, has this behaviour caused a problem, or is it just something observed that seems incorrect?

[clrudolphi]

I've asked the Cucumber team (via a Gherkin project bug report). I'll report back what they say.

[clrudolphi]

I've asked the Cucumber team (via a Gherkin project bug report). I'll report back what they say.

While the Cucumber implementations normalize multi-line descriptions to be separated by '\n', the .net predecessor(s) to Reqnroll did not. This behavior has been in the code since at least 2017.
The conformance tests over in the Gherkin project (and in Reqnroll's Formatters tests) explicitly ignore differences in line-endings. So this behavior is not something that 'slipped through' via over sight.

As @Code-Grump asked earlier, what's the harm?

[gasparnagy]

As @Code-Grump asked earlier, what's the harm?

I think the harm is clear: testing should be deterministic and it should not depend on the platform the test is built or executed on. According to the Gherkin concept, the feature/scenario/etc descriptions should not be considered as part of the test, so for those the difference is really cosmetic. The more problematic is the multi-line doc string, because that is test data. I wonder if the same problem applies to that one as well. But in any case, I would classify that more of an inconvenience.

But even if we consider it as harmful, it's a good question what we should do as a strategy. As a background, 70% of our test executions are on Windows.

Possible options:

1. Keep it as it is and document that the line endings has to be normalized by the test code if that is relevant. As far as I understand currently we follow the rule: "Use CRLF vs LF depending on the OS where it was compiled"
2. Apply the Cucumber implementation pattern: change everything to \n, but then make the usage inconvenient and breaking for 70% of our users
3. Come up with an own, consistent model
   a) Use CRLF vs LF depending on the OS where it is executed (not much better then the current situation, because you usually run the test on the same OS as it was compiled, and not deterministic either)
   b) Use CRLF vs LF depending on the source file line ending (so it might be even mixed! - in most cases this would end up being the same as what we have currently because by default git checks out the line endings according to the OS). This is pretty complicated though, especially because currently the Gherkin token scanner does not preserve the line ending as far as I remember.
   c) you name it

To be honest I don't like the Cucumber approach (I was never fond of the *nix-supremacy). But if we would rule that out, then I see no strong point changing one kind-of consistent model to another one, so I tend to prefer option 1.

[Code-Grump]

I think input normalisation is a standard part of most test suites. If I'm reading a text file for use in a test case, such as when I want to ensure a produced file matches a specification, I have to normalise the line endings of my expected text based on the runtime platform.

From my perspective, I might actually prefer the system to not normalize for me, since it's going to be more consistent with other file handling.

[gasparnagy]

From my perspective, I might actually prefer the system to not normalize for me, since it's going to be more consistent with other file handling.

So option 3/b?

[Code-Grump]

No, sorry; option 1: let the user decide how they want to handle line-endings for themselves.

[304NotModified]

Also voting for option 1

[clrudolphi]

Also agree on option 1 (keep it as-is).
Is everyone OK with marking this issue as 'won't fix' and closing it?

[thatpaulschofield]

I would like for the generated line endings to at least be configurable. My team has Mac and Windows users, and it's inconvenient for all my *.feature.cs to change after I pull and build.

I don't advocate for changing the default behavior, but perhaps allow a config setting to specify the line ending in generated files?