Add dotnet implementation of the htmlformatter #352

clrudolphi · 2025-02-16T23:54:18Z

Port of the Java implementation to .NET.

🤔 What's changed?

Added .NET implementation of htmlformatter and test suite, based port of the Java implementation.
Added lines to the global Make file to push the javascript build artifacts to the dotnet Resources folder.

⚡️ What's your motivation?

Intended for inclusion with Reqnroll reporting.

🏷️ What kind of change is this?

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

♻️ Anything particular you want feedback on?

Will need assistance in getting the Nuget package publishing set-up.

📋 Checklist:

I agree to respect and uphold the Cucumber Community Code of Conduct
I've changed the behaviour of the code
- [X ] 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
- [X ] I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

Initial Commit

Added Unreleased note about the addition of the .NET impl

.gitignore

mpkorstanje · 2025-02-17T09:36:03Z

dotnet/html-formatter/htmlformatter/htmlformatter.csproj

+  </ItemGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Cucumber.Messages" Version="27.0.2" />


Is it possible to use a version range here? The html formatter is fairly backwards compatible with older messages.

I put in a range to go back to R26. That was when I updated the Messages code generation approach to use the current system (retiring the old protobuf method). Assuming there was a risk that I broke something there, that is as far back as I'm comfortable.

I had to correct that to only r27.0 as the minimum. R27 introduced a new enumeration type (HookType) and the serializer needs to know about all such enumerations for proper conversion.

mpkorstanje

Looks good, on first quick review I don't see anything too surprising.

Do you need help setting up automated tests on Github? The messages and or Gherkins repos might provide inspiration

davidjgoss

More of a question, but the root of the .NET impl here is dotnet/htmlformatter, where with Gherkin for example it's just under dotnet (basing this on where the .sln file is in each one). Could/should we squash this up to match Gherkin's structure?

CHANGELOG.md

clrudolphi · 2025-02-17T14:25:19Z

More of a question, but the root of the .NET impl here is dotnet/htmlformatter, where with Gherkin for example it's just under dotnet (basing this on where the .sln file is in each one). Could/should we squash this up to match Gherkin's structure?

Hmm, there are some inconsistencies then across the Cucumber/dotnet landscape. In Messages, the source and .sln are in ./dotnet/Cucumber.Messages (that is, not directly under the dotnet folder).

Which would you prefer? The Gherkin structure is much older. I created the Messages one. So I don't mind changing both.

Corrected .gitignore files and CHANGELOG.md Reference to Messages nuget package updated to use a range specifier (going back to r26)

mpkorstanje · 2025-02-17T15:52:14Z

Hmm, there are some inconsistencies then across the Cucumber/dotnet landscape. In Messages, the source and .sln are in ./dotnet/Cucumber.Messages (that is, not directly under the dotnet folder).

Not sure I'm following that one.

There is

So I would expect there to be something like dotnet/Html.Formatter.sln

clrudolphi · 2025-02-17T15:53:14Z

Hmm, there are some inconsistencies then across the Cucumber/dotnet landscape. In Messages, the source and .sln are in ./dotnet/Cucumber.Messages (that is, not directly under the dotnet folder).

Not sure I'm following that one.

There is

https://github.com/cucumber/gherkin/blob/main/dotnet/Gherkin.sln

https://github.com/cucumber/messages/blob/main/dotnet/Cucumber.Messages.sln

So I would expect there to be something like dotnet/Html.Formatter.sln

Oh, I see. I thought you were commenting the directory structure per se; not merely the location of the .sln file. I'll fix that.

Removed the intermediary htmlformatter folder in order to match other Cucumber project's structures. Corrected nuget dependency range on Messages to [27.0,28.0).

clrudolphi · 2025-02-17T16:43:41Z

I'll fix that.

Fixed. Intermediary folder removed.

Updated the .csproj file to include information for Nuget publication of the htmlformatter assembly.

Adopted .github/workflow/*.yml from the Messages repo for .NET build and publication.

clrudolphi · 2025-02-17T17:42:35Z

Added CI build files in .github/workflows, based upon what was used in Messages.
This fails as we need a .snk (Strong Name Key) file for html-formatter.

Does the github build system invoke the Make before invoking each of the individual platform github build/test yml workflows? (just want to ensure the generated assets are in place...)

.github/workflows/release-nuget.yaml

mpkorstanje · 2025-02-17T18:05:44Z

dotnet/html-formatter.sln

Mostly a question: What are the .net standards for file names? The gherkin and messages projects seem to be using pascal case for these files and . in place of non-az characters.

More of a convention than a standard, yes. I will change them.
In terms of project naming, do you want me to follow the lead of the java implementation and simply call it Htmlformatter? Or should I follow the lead of Messages and call it Cucumber.Htmlformatter?

That depends!

For Java the full name is io.cucumber:html-formatter.

Looks like NuGet doesn't have namespaces. So the produced artifact should be Cucumber.HtmlFormatter.

Then I would expect the project files to follow the artifact name. Unless there is a strong convention in the .Net world that recommends otherwise. In which case the files in our other projects should be renamed.

Ok. Changed to Cucumber.Htmlformatter.

Not in Pascal case?

mpkorstanje · 2025-02-17T18:14:32Z

This fails as we need a .snk (Strong Name Key) file for html-formatter.

I get the impression we are following this advice:

If you are an open-source developer and you want the identity benefits of a strong-named assembly for better compatibility with .NET Framework, consider checking in the private key associated with an assembly to your source control system.

But I'm not sure, I can't read the .snkfiles from Gherkin and Messages to compare.

Changed capitalization of names of files to match convention used in other projects. Added a .snk file for the nuget publishing.

clrudolphi · 2025-02-17T20:00:04Z

This fails as we need a .snk (Strong Name Key) file for html-formatter.

I get the impression we are following this advice:

If you are an open-source developer and you want the identity benefits of a strong-named assembly for better compatibility with .NET Framework, consider checking in the private key associated with an assembly to your source control system.

But I'm not sure, I can't read the .snkfiles from Gherkin and Messages to compare.

Talked to Gaspar, and checked in a .snk file.

Further correction based upon feedback

Again; to make it Pascal cased.

clrudolphi

the test-dotnet is failing with a pathing problem. Would you have time to take a look?
The project file "/home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatter/Cucumber.HtmlFormatter.csproj" was not found.

dotnet/HtmlformatterTest/Cucumber.HtmlformatterTest.csproj

mpkorstanje · 2025-02-17T20:37:14Z

@charlierudolph are you using a case insensitive file system?

clrudolphi · 2025-02-17T20:43:45Z

@charlierudolph are you using a case insensitive file system?

Well that sucks.

That doesn't match what is in my local repo.

I'm on Windows if that matters.

mpkorstanje · 2025-02-17T20:56:42Z

Then your filesystem is case insensitive by default.

https://learn.microsoft.com/en-us/windows/wsl/case-sensitivity

I'm going to modify the files. Then you'll probably want to delete your repo and clone it again.

mpkorstanje · 2025-02-17T22:05:46Z

Now it works.

Just some warnings and complaints in the output.

Warning: /home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatter/MessagesToHtmlWriter.cs(30,48): warning CS8625: Cannot convert null literal to non-nullable reference type. [/home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatter/Cucumber.HtmlFormatter.csproj]
Warning: /home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatter/MessagesToHtmlWriter.cs(39,62): warning CS8625: Cannot convert null literal to non-nullable reference type. [/home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatter/Cucumber.HtmlFormatter.csproj]
  Cucumber.HtmlFormatter -> /home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatter/bin/Debug/netstandard2.0/Cucumber.HtmlFormatter.dll
  The package Cucumber.HtmlFormatter.21.9.0 is missing a readme. Go to https://aka.ms/nuget/authoring-best-practices/readme to learn why package readmes are important.
  Successfully created package '/home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatter/bin/Debug/NuGet/Cucumber.HtmlFormatter.21.9.0.nupkg'.
Warning: /home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatterTest/CucumberMessageEnumConverter.cs(20,29): warning CS8605: Unboxing a possibly null value. [/home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatterTest/Cucumber.HtmlFormatterTest.csproj]
Warning: /home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatterTest/CucumberMessageEnumConverter.cs(31,46): warning CS8604: Possible null reference argument for parameter 'key' in 'bool Dictionary<string, T>.TryGetValue(string key, out T value)'. [/home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatterTest/Cucumber.HtmlFormatterTest.csproj]

clrudolphi · 2025-02-17T22:06:28Z

Now it works.

Just some warnings and complaints in the output.

Warning: /home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatter/MessagesToHtmlWriter.cs(30,48): warning CS8625: Cannot convert null literal to non-nullable reference type. [/home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatter/Cucumber.HtmlFormatter.csproj]
Warning: /home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatter/MessagesToHtmlWriter.cs(39,62): warning CS8625: Cannot convert null literal to non-nullable reference type. [/home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatter/Cucumber.HtmlFormatter.csproj]
  Cucumber.HtmlFormatter -> /home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatter/bin/Debug/netstandard2.0/Cucumber.HtmlFormatter.dll
  The package Cucumber.HtmlFormatter.21.9.0 is missing a readme. Go to https://aka.ms/nuget/authoring-best-practices/readme to learn why package readmes are important.
  Successfully created package '/home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatter/bin/Debug/NuGet/Cucumber.HtmlFormatter.21.9.0.nupkg'.
Warning: /home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatterTest/CucumberMessageEnumConverter.cs(20,29): warning CS8605: Unboxing a possibly null value. [/home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatterTest/Cucumber.HtmlFormatterTest.csproj]
Warning: /home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatterTest/CucumberMessageEnumConverter.cs(31,46): warning CS8604: Possible null reference argument for parameter 'key' in 'bool Dictionary<string, T>.TryGetValue(string key, out T value)'. [/home/runner/work/html-formatter/html-formatter/dotnet/HtmlFormatterTest/Cucumber.HtmlFormatterTest.csproj]

EDIT: I've addressed the compilation warnings.
I'll address the read.me nag tomorrow.

Added README.md and included it in the nuget package definition.

gasparnagy

I'm fine with it generally. I have made a few minor code-style related comments.

gasparnagy · 2025-02-19T08:12:35Z

dotnet/HtmlFormatter/Cucumber.HtmlFormatter.csproj

+    <None Remove="Resources\index.mustache.html" />
+    <None Remove="Resources\main.css" />
+    <None Remove="Resources\main.js" />
+    <None Remove="Resources\main.js.LICENSE.txt" />


As far as I know, you don't have to remove the items from the None group (because it is not doing anything anyway)

dotnet/HtmlFormatter/Cucumber.HtmlFormatter.csproj

dotnet/HtmlFormatter/MessagesToHtmlWriter.cs

dotnet/HtmlFormatterTest/CucumberMessageEnumConverter.cs

dotnet/HtmlFormatter/JsonInHtmlWriter.cs

gasparnagy

I don't know if anything is still open from the others, but from my perspective this is good to go, thx!

clrudolphi added 2 commits February 16, 2025 17:47

Initial Commit of dotnet implementation of the htmlformatter

cc46ef4

Initial Commit

Updated CHANGELOG

79828ba

Added Unreleased note about the addition of the .NET impl

mpkorstanje self-requested a review February 17, 2025 08:26

mpkorstanje reviewed Feb 17, 2025

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

mpkorstanje reviewed Feb 17, 2025

View reviewed changes

davidjgoss reviewed Feb 17, 2025

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Clean-ups per PR comments

cf6eadb

Corrected .gitignore files and CHANGELOG.md Reference to Messages nuget package updated to use a range specifier (going back to r26)

Structure update

91212a8

Removed the intermediary htmlformatter folder in order to match other Cucumber project's structures. Corrected nuget dependency range on Messages to [27.0,28.0).

clrudolphi added 2 commits February 17, 2025 11:34

Updated csproj for Nuget publication

14eea0d

Updated the .csproj file to include information for Nuget publication of the htmlformatter assembly.

Set up CI

d41ee8a

Adopted .github/workflow/*.yml from the Messages repo for .NET build and publication.

Make workflows match

7266d78

mpkorstanje reviewed Feb 17, 2025

View reviewed changes

.github/workflows/release-nuget.yaml Show resolved Hide resolved

mpkorstanje reviewed Feb 17, 2025

View reviewed changes

Refactoring

53b0eb0

Changed capitalization of names of files to match convention used in other projects. Added a .snk file for the nuget publishing.

clrudolphi added 2 commits February 17, 2025 14:06

Name normalization

c87a900

Further correction based upon feedback

Naming corrections

71726c8

Again; to make it Pascal cased.

clrudolphi commented Feb 17, 2025

View reviewed changes

mpkorstanje reviewed Feb 17, 2025

View reviewed changes

dotnet/HtmlformatterTest/Cucumber.HtmlformatterTest.csproj Outdated Show resolved Hide resolved

mpkorstanje added 8 commits February 17, 2025 22:00

Rename to PascalCase

4f90d27

Make sln match Gherkin a bit more

43149fc

Debug ci

e5b4304

Debug ci

9bdce41

Rename files

1e65b2d

Remove debug code

80da61f

Fix make

d849829

Fix make

dd0fd37

clrudolphi added 2 commits February 17, 2025 17:12

Adjustments to address compilation warnings.

f1bdf97

Add README.md

264c17a

Added README.md and included it in the nuget package definition.

clrudolphi marked this pull request as ready for review February 18, 2025 22:22

clrudolphi requested a review from gasparnagy February 18, 2025 22:23

gasparnagy requested changes Feb 19, 2025

View reviewed changes

mpkorstanje changed the title ~~Initial Commit of dotnet implementation of the htmlformatter~~ Add dotnet implementation of the htmlformatter Feb 19, 2025

Changes per code review

8a33c08

gasparnagy approved these changes Feb 19, 2025

View reviewed changes

mpkorstanje self-requested a review February 19, 2025 23:22

mpkorstanje approved these changes Feb 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add dotnet implementation of the htmlformatter #352

Add dotnet implementation of the htmlformatter #352

clrudolphi commented Feb 16, 2025

mpkorstanje Feb 17, 2025

clrudolphi Feb 17, 2025

clrudolphi Feb 17, 2025

mpkorstanje left a comment •

edited

Loading

davidjgoss left a comment

clrudolphi commented Feb 17, 2025 •

edited

Loading

mpkorstanje commented Feb 17, 2025

clrudolphi commented Feb 17, 2025

clrudolphi commented Feb 17, 2025

clrudolphi commented Feb 17, 2025 •

edited

Loading

mpkorstanje Feb 17, 2025

clrudolphi Feb 17, 2025

mpkorstanje Feb 17, 2025

clrudolphi Feb 17, 2025

mpkorstanje Feb 17, 2025

clrudolphi Feb 17, 2025

mpkorstanje commented Feb 17, 2025 •

edited

Loading

clrudolphi commented Feb 17, 2025

clrudolphi left a comment

mpkorstanje commented Feb 17, 2025

clrudolphi commented Feb 17, 2025

mpkorstanje commented Feb 17, 2025

mpkorstanje commented Feb 17, 2025 •

edited

Loading

clrudolphi commented Feb 17, 2025 •

edited

Loading

gasparnagy left a comment

gasparnagy Feb 19, 2025

clrudolphi Feb 19, 2025

gasparnagy left a comment

Add dotnet implementation of the htmlformatter #352

Are you sure you want to change the base?

Add dotnet implementation of the htmlformatter #352

Conversation

clrudolphi commented Feb 16, 2025

🤔 What's changed?

⚡️ What's your motivation?

🏷️ What kind of change is this?

♻️ Anything particular you want feedback on?

📋 Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpkorstanje left a comment • edited Loading

Choose a reason for hiding this comment

davidjgoss left a comment

Choose a reason for hiding this comment

clrudolphi commented Feb 17, 2025 • edited Loading

mpkorstanje commented Feb 17, 2025

clrudolphi commented Feb 17, 2025

clrudolphi commented Feb 17, 2025

clrudolphi commented Feb 17, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpkorstanje commented Feb 17, 2025 • edited Loading

clrudolphi commented Feb 17, 2025

clrudolphi left a comment

Choose a reason for hiding this comment

mpkorstanje commented Feb 17, 2025

clrudolphi commented Feb 17, 2025

mpkorstanje commented Feb 17, 2025

mpkorstanje commented Feb 17, 2025 • edited Loading

clrudolphi commented Feb 17, 2025 • edited Loading

gasparnagy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gasparnagy left a comment

Choose a reason for hiding this comment

mpkorstanje left a comment •

edited

Loading

clrudolphi commented Feb 17, 2025 •

edited

Loading

clrudolphi commented Feb 17, 2025 •

edited

Loading

mpkorstanje commented Feb 17, 2025 •

edited

Loading

mpkorstanje commented Feb 17, 2025 •

edited

Loading

clrudolphi commented Feb 17, 2025 •

edited

Loading