Repository navigation
Join test names in NUnit and JUnit summaries with joinWith - #535
Conversation
The NUnit and JUnit result writers passed the test name, a string list, straight to XAttribute. F# list ToString() truncates after three items, so nested tests were written as "[a; b; c; ... ]" and many tests shared the same name in the report. The writers now join the name with the configured JoinWith. The new writeNUnitSummaryWith and writeJUnitSummaryWith take the separator; the existing writeNUnitSummary and writeJUnitSummary keep their signatures and join with ".". AddNUnitSummary and AddJUnitSummary read joinWith from the final config when the summary is written, through the new appendSummaryHandlerWithConfig. This makes the result independent of where --join-with appears relative to --nunit-summary or --junit-summary on the command line.
|
Thanks a bunch for merging my PRs @farlee2121! |
|
I'm running the release task currently |
|
Also, I'd be glad to hear your thoughts on how we can improve the config. It's been something I've meant to get to for a while. My attempts to improve code location have kept me occupied longer than expected Speaking of which, I did an experiment trying to move the code location work you did for Ionide into a package. |
|
These changes should be up on nuget with version 11.1.1 |
|
Thanks for the quick release! On config: the friction I hit in FSAC#1554 was mostly around timeouts and cancellation. On the location experiment: I can't take any credit for the FSAC side. The AST walker has been in FSAC for years and was written and maintained by others. I only got involved very recently and I'm just trying a few things. My one change there (#1553) was about which files get parsed for test detection, and with which parsing options. That said, it's why I agree that build time is the right place for this: MSBuild knows the compile items, defines and language version, and guessing them from the assembly will drift. Since only the untyped tree is needed, Fantomas.FCS might be a lighter and more convenient dependency than all of FCS. And as name matching only works for literal test names, I'd keep the Cecil locator as the fallback. |
On configPart of the problem i've experienced is the close mirroring between ExpectoConfig and CLI args. For example, when I previously tried to transition us away from a static custom logger to Microsoft.Extensions.Logging, I got mired in dependencies between items in ExpectoConfig made it rather difficult to swap the printer or logger without breaking other behaviors (i.e. color, making sure the printer used the logger, making sure it wasn't all overwritten like you observed with the stress / summary behavior) My thought is that we need to break up ExpectoConfig. ExpectoConfig would be split up. We'd delineate printer settings (verbosity, color), the actual composed printer instance, run modes and their child settings (i.e. stress, FsCheck), etc. This would allow us to be more explicit about what depends on what and which options are mutually exclusive, making it more clear how to map console args. It also sets up Expecto's core to be less tied to assumptions of the console runner. On code locationI'll have to take a look at Fantomas.FCS. FCS is definitely a heavy dependency. I've also experimented with using caller info attributes to decide code locations at compile time. But it would require all of the I've also looked at improving the reflection-based approach. But finding testList nodes that way is rough and finding async tests would require a pretty significant breaking change |
The NUnit and JUnit result writers passed the test name, a string list, straight to XAttribute. F# list ToString() truncates after three items, so nested tests were written as "[a; b; c; ... ]" and many tests shared the same name in the report.
The writers now join the name with the configured JoinWith. The new writeNUnitSummaryWith and writeJUnitSummaryWith take the separator; the existing writeNUnitSummary and writeJUnitSummary keep their signatures and join with ".".
AddNUnitSummary and AddJUnitSummary read joinWith from the final config when the summary is written, through the new appendSummaryHandlerWithConfig. This makes the result independent of where --join-with appears relative to --nunit-summary or --junit-summary on the command line.
Known gap, not addressed here: result files can be dropped silently
--stressand the code-onlyPrinterargument replace the whole printer. Summary handlers, including the ones--nunit-summaryand--junit-summaryadd, live inside the printer'ssummaryfunction, so they are lost when one of those arguments is applied after them:--junit-summary out.xml --stress 1: exit code 0, no warning, no file written.--stress 1 --junit-summary out.xml: the file is written.runTestsWithCLIArgs [NUnit_Summary "..."] args(as inExpecto.Tests/Main.fs) followed by--stresson the command line always loses the file, whatever order the user types.Nothing tells the user. The run reports success, the README does not mention the interaction, and an older file at the same path is left in place, so CI can pick up results from a previous run.
Fixes #534
Why it is not fixed in this PR:
ExpectoConfig) removes the problem properly, but adding a record field breaks code that buildsExpectoConfigwithoutdefaultConfig.Happy to open a separate issue to pick an approach.