fix(cli): output more detailed information for steps when using JUnit reporter

[magurotuna]

This patch gets JUnit reporter to output more detailed information for test steps (subtests).

Issue with previous implementation

In the previous implementation, the test hierarchy was represented using several XML tags like the following:

• <testsuites> corresponds to the entire test (one execution of deno test has exactly one <testsuites> tag)
• <testsuite> corresponds to one file, such as main_test.ts
• <testcase> corresponds to one Deno.test(...)
• <property> corresponds to one t.step(...)

This structure describes the test layers but one problem is that <property> tag is used for any use cases so some tools that can ingest a JUnit XML file might not be able to interpret <property> as subtests.

How other tools address it

Some of the testing frameworks in the ecosystem address this issue by fitting subtests into the <testcase> layer. For instance, take a look at the following Go test file:

package main_test

import "testing"

func TestMain(t *testing.T) {
  t.Run("child 1", func(t *testing.T) {
    // OK
  })

  t.Run("child 2", func(t *testing.T) {
    // Error
    t.Fatal("error")
  })
}

Running gotestsum, we can get the output like this:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites tests="3" failures="2" errors="0" time="1.013694">
	<testsuite tests="3" failures="2" time="0.510000" name="example/gosumtest" timestamp="2024-03-11T12:26:39+09:00">
		<properties>
			<property name="go.version" value="go1.22.1 darwin/arm64"></property>
		</properties>
		<testcase classname="example/gosumtest" name="TestMain/child_2" time="0.000000">
			<failure message="Failed" type="">=== RUN   TestMain/child_2&#xA;    main_test.go:12: error&#xA;--- FAIL: TestMain/child_2 (0.00s)&#xA;</failure>
		</testcase>
		<testcase classname="example/gosumtest" name="TestMain" time="0.000000">
			<failure message="Failed" type="">=== RUN   TestMain&#xA;--- FAIL: TestMain (0.00s)&#xA;</failure>
		</testcase>
		<testcase classname="example/gosumtest" name="TestMain/child_1" time="0.000000"></testcase>
	</testsuite>
</testsuites>

This output shows that nested test cases are squashed into the <testcase> layer by treating them as the same layer as their parent, TestMain. We can still distinguish nested ones by their name attributes that look like TestMain/<subtest_name>.

As described in #22795, vitest solves the issue in the same way as gotestsum.

One downside of this would be that one test failure that happens in a nested test case will end up being counted multiple times, because not only the subtest but also its wrapping container(s) are considered to be failures. In fact, in the gotestsum output above, TestMain/child_2 failed (which is totally expected) while its parent, TestMain, was also counted as failure. As #20273 (comment) pointed out, there is a test runner that offers flexibility to prevent this, but I personally don't think the "duplicate failure count" issue is a big deal.

How to fix the issue in this patch

This patch fixes the issue with the same approach as gotestsum and vitest.
More specifically, nested test cases are put into the <testcase> level and their names are now represented as squashed test names concatenated by > (e.g. parent 2 > child 1 > grandchild 1). This change also allows us to put a detailed error message as <failure> tag within the <testcase> tag, which should be handled nicely by third-party tools supporting JUnit XML.

Extra fix

Also, file paths embedded into XML outputs are changed from absolute path to relative path, which is helpful when running the test suites in several different environments like CI.

Resolves #22795

[magurotuna]

For reviewers: this is just a refactor; ToString trait is automatically implemented for all types that implement std::fmt::Display, so basically it's recommended to implement std::fmt::Display

[bartlomieju]

One downside of this would be that one test failure that happens in a nested test case will end up being counted multiple times, because not only the subtest but also its wrapping container(s) are considered to be failures. In fact, in the gotestsum output above, TestMain/child_2 failed (which is totally expected) while its parent, TestMain, was also counted as failure. As #20273 (comment) pointed out, there is a test runner that offers flexibility to prevent this, but I personally don't think the "duplicate failure count" issue is a big deal.

I agree, this is a reasonable trade-off.

[bartlomieju]

Great improvement. I have a couple of requests, but overall LGTM.

Pinging @littledivy for a second pair of eyes and to ensure npm_smoke_tests repo won't suddenly break because of this change.

[bartlomieju]

This will eventually fail and panic for user; can you pass down cwd from CliOptions so it's handled gracefully?

[magurotuna]

I brought this from the existing code of PrettyReporter:

deno/cli/tools/test/reporters/pretty.rs

Line 40 in b6e44f9

cwd: Url::from_directory_path(std::env::current_dir().unwrap()).unwrap(),

Should we also fix these ones as well?

[bartlomieju]

Yes, I think it would be better to fix both then

[magurotuna]

Sure! I noticed that all the test reporters have this code, so I'll fix all of them

[magurotuna]

OK, it's done 752db03

[bartlomieju]

Maybe pre-allocate to something like 256 to avoid excessive reallocations?

[magurotuna]

Good point! fixed in 14d89d2