Skip to content

Join test names in NUnit and JUnit summaries with joinWith - #535

Merged
farlee2121 merged 1 commit into
haf:mainfrom
nojaf:t3code/853db549
Oct 9, 2026
Merged

farlee2121 merged 1 commit into
haf:mainfrom
nojaf:t3code/853db549

Conversation

@nojaf

@nojaf nojaf commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

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

--stress and the code-only Printer argument replace the whole printer. Summary handlers, including the ones --nunit-summary and --junit-summary add, live inside the printer's summary function, 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.
  • Arguments passed in code are applied before the command line, so runTestsWithCLIArgs [NUnit_Summary "..."] args (as in Expecto.Tests/Main.fs) followed by --stress on 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:

  • It predates this change and is unrelated to how names are formatted. This PR neither causes nor worsens it.
  • The fix needs a design decision. Applying printer-replacing arguments before summary handlers is small and keeps the API, but adds another ordering rule. Keeping summary handlers outside the printer (a list on ExpectoConfig) removes the problem properly, but adding a record field breaks code that builds ExpectoConfig without defaultConfig.

Happy to open a separate issue to pick an approach.

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.
@farlee2121
farlee2121 merged commit 89f3419 into haf:main Oct 9, 2026
@nojaf

nojaf commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks a bunch for merging my PRs @farlee2121!
Could we get a release for this maybe?
These things surfaced while working on ionide/FsAutoComplete#1554
Would be great if I could have those improvements in there as well.

@farlee2121

Copy link
Copy Markdown
Collaborator

I'm running the release task currently

@farlee2121

Copy link
Copy Markdown
Collaborator

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.
I got it running with some hacks. If we moved forward with it we'd probably want to run it at build-time so we know we have access to the code instead of guessing where the code is from the assembly

https://github.com/haf/expecto/tree/ast-location-discovery

@farlee2121

Copy link
Copy Markdown
Collaborator

These changes should be up on nuget with version 11.1.1

@nojaf

nojaf commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the quick release!

On config: the friction I hit in FSAC#1554 was mostly around timeouts and cancellation. Test.timeout reports a timeout but leaves the test running next to the ones after it, so I ended up writing my own cancel-on-timeout wrapper. A cancelled run also exits with 0. Happy to open issues for those. Did you mean ExpectoConfig itself, or the CLI args side?

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.

@farlee2121

farlee2121 commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

On config

Part 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 location

I'll have to take a look at Fantomas.FCS. FCS is definitely a heavy dependency.
We'll also need to keep the reflection-based location for C# users.

I've also experimented with using caller info attributes to decide code locations at compile time. But it would require all of the testCase and similar functions to be type members instead. I haven't checked, but I suspect it won't work with partial application.

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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--junit-summary and --nunit-summary write test names as truncated F# lists

2 participants