testing: add Name to track file and line of test case declaration

[dsnet]

In Go, it is very common to use table-driven tests:

tests := struct {
    name string
    input T
    ...
} {{
    name: "Foo",
    ...,
}, {
    name: "Bar",
    ...,
}
... // maybe dozens or hundreds more cases
}
for _, tt := range tests {
    t.Run(tt.name, func(t *testing.T) {
        got, err := fizz(tt.input)
        if err != nil {
            t.Fatalf("fizz error: %v", err) // my_test.go:1234
        }
    })
}

When this test fails, it prints with something like:

--- FAIL: Test/Bar (0.00s)
    my_test.go:1234: fizz error: the world blew up

• The my_test.go:1234 tells us where in the test logic this failed.
• The Test/Bar name tells us which test case failed.

Most code editors today identify source.go:1234 strings and automatically provide the ability to jump to that source code location. This is really helpful for jumping to the execution logic that failed, but is not helpful for jumping to the test data that caused the failure. It is impossible for editor tooling to automatically correlate the the test name (e.g., Test/Bar) with the test case in the code since the association between the two can be determined by arbitrary Turing-complete logic.

I propose the following API in the testing package:

// NameFileLine is a name combined with a file and line number.
type NameFileLine struct { ... }

// Name constructs a NameFileLine.
// It annotates the name with the file and line number of the caller.
func Name(name string) NameFileLine

// RunName runs f as a subtest of t called name.
func (t *T) RunName(name NameFileLine, f func(t *testing.T))

// RunName runs f as a subtest of b called name.
func (b *B) RunName(name NameFileLine, f func(b *testing.B))

Using this API, the example above would be changed as:

  tests := struct {
-     name string
+     name testing.NameFileLine
      input T
      ...
  } {{
-     name: "Foo",
+     name: testing.Name("Foo"),
      ...,
  }, {
-     name: "Bar",
+     name: testing.Name("Bar"), // my_test.go:321
      ...,
  }
  ... // maybe dozens or hundreds more cases
  }
  for _, tt := range tests {
-     t.Run(tt.name, func(t *testing.T) {
+     t.RunName(tt.name, func(t *testing.T) {
          got, err := fizz(tt.input)
          if err != nil {
              t.Fatalf("fizz error: %v", err) // my_test.go:1234
          }
      })
  }

• We call testing.Name in every test case, which captures file and line information about where the test case was declared.
• We call testing.T.RunName and pass it the testing.TestName so that the subtest knows what test case is associated with this subtest.

Thus, the test output would be something like:

--- FAIL: Test/Bar (0.00s)
    my_test.go:321: my_test.go:1234: fizz error: the world blew up

• The my_test.go:321 tells us where the test data was declared.
• The my_test.go:1234 tells us where in the test logic this failed.

Now, we can click on my_test.go:321 in our code editors and it will take us directly to the test case declaration.

[randall77]

tests := struct {
    name string
    fileLine string
    input T
} {
    name: "foo",
    fileLine: fileLine(),
    ...
}

func fileLine() string {
    _, file, line, _ := runtime.Caller(1)
    return file + ":" + line
}

This is similar to the workaround we use in the stdlib, e.g., reflect.verifyGCBits.

[randall77]

That said, it would be nice to have something ergonomic built in to testing. I wish testing.Run took an any instead of a string as the name, so we didn't have to introduce another method on Testing to enable this.

[dsnet]

To avoid declaring new RunName methods, we could do something like:

  for _, tt := range tests {
-     t.Run(tt.name, func(t *testing.T) {
+     t.Run(tt.name.String(), func(t *testing.T) {
          ...
      })
  }

where testing.NameFileLine.String prints in a special syntax recognized by testing.T.Run and testing.B.Run. However, this might be considered a breaking change.

[rsc]

It seems like there are two things going on here:

1. The trick about testing.Name recording where it was called from.
2. The ability to add file:line annotations to test failures.

It seems like we could keep t.Run the same, pass tt.name.String() to it, and then separately have a

t.AddFileLine(tt.name.FileLine())

call at the start of the function body. Then other sources of file:line (like testscript failures) can hook into this too.

[rsc]

This proposal has been added to the active column of the proposals project and will now be reviewed at the weekly proposal review meetings. — rsc for the proposal review group

[dnephin]

I have encountered this problem a few times. I'll share the workaround I came up with in case it helps this proposal in some way.

Using t.Helper and a run function I was able to have the tests emit at least 2 file and line number outputs: one for the first line of the test case definition, and one from the test logic function where the failure happened.

To make this work, the traditional test case "table" has to be modified a bit.

• the logic of the test is defined at the top in a run function, instead of at the end.
• instead of a slice or map of test cases, each one calls run. It's a few extra characters per test case.

Using the example test from the description, it might look like this:

type testCase struct {
    name string
    input T
    ...
}

run := func(t *testing.T, tc testCase) {
    t.Helper()
    t.Log("case:", tc.name)
    t.Run(name, func(t *testing.T) {
        got, err := fizz(tt.input)
        if err != nil {
            t.Fatalf("fizz error: %v", err) // my_test.go:1234
        }
    })
}

run(t, testCase{
    name: "Foo",
    ...,
}
run(t, testCase{
    name: "Bar",
    ...,
}
... // maybe dozens or hundreds more cases

Definitely not as elegant as the fileLine workaround. t.AddFileLine would be a great addition!

[rsc]

I'd still like to understand better whether we can separate out the two different features being added, as I noted in https://github.com/golang/go/issues/52751#issuecomment-1124087356. Any thoughts, @dsnet?

[AlexanderYastrebov]

Since new type is introduced maybe there is no need to have String and AddFileLine methods but just a Run wrapper:

package go52751

import (
    "fmt"
    "runtime"
    "testing"
)

type TC struct {
    name     string
    location string
}

func (tc *TC) Run(t *testing.T, tf func(t *testing.T)) bool {
    t.Helper()
    return t.Run(tc.name, func(t *testing.T) {
        t.Helper()
        // this should use internal undecorated logging to achieve desired output
        t.Logf("Test case %q defined at %s", tc.name, tc.location)
        tf(t)
    })
}

func testingCase(name string) *TC {
    _, file, line, _ := runtime.Caller(1)
    return &TC{name, fmt.Sprintf("%s:%d", file, line)}
}

func TestFileLine(t *testing.T) {
    tests := []struct {
        tc    *TC
        input string
    }{{
        tc:    testingCase("Foo"),
        input: "x",
    }, {
        tc:    testingCase("Bar"),
        input: "",
    },
    }
    for _, tt := range tests {
        tt.tc.Run(t, func(t *testing.T) {
            if tt.input == "" {
                t.Fatalf("input error")
            }
        })
    }
}

--- FAIL: TestFileLine (0.00s)
    --- FAIL: TestFileLine/Bar (0.00s)
        case_test.go:42: Test case "Bar" defined at /tmp/go52751/case_test.go:37
        case_test.go:44: input error
FAIL
FAIL    command-line-arguments  0.001s
FAIL

[dsnet]

@AlexanderYastrebov, you would need two different Run methods since the method must taking in either a func(*testing.T) or a func(*testing.B). It's not clear that it's a cleaner API than what was originally proposed.

[AlexanderYastrebov]

@dsnet

diff --git a/src/testing/testing.go b/src/testing/testing.go
index ec2d864822..dc1bfc301e 100644
--- a/src/testing/testing.go
+++ b/src/testing/testing.go
@@ -529,6 +529,8 @@ type common struct {
        tempDir    string
        tempDirErr error
        tempDirSeq int32
+
+       cases map[string]string
 }

 // Short reports whether the -test.short flag is set.
@@ -1451,6 +1453,27 @@ func tRunner(t *T, fn func(t *T)) {
        t.mu.Unlock()
 }

+func (t *T) Case(name string) string {
+       _, file, line, _ := runtime.Caller(1)
+       location := fmt.Sprintf("%s:%d\n", file, line)
+
+       t.mu.Lock()
+       if t.cases == nil {
+               t.cases = make(map[string]string)
+       }
+       uniqName := name
+       for i := 1; ; i++ {
+               if _, ok := t.cases[uniqName]; !ok {
+                       break
+               }
+               uniqName = fmt.Sprintf("%s#%d", name, i)
+       }
+       t.cases[uniqName] = location
+       t.mu.Unlock()
+
+       return uniqName
+}
+
 // Run runs f as a subtest of t called name. It runs f in a separate goroutine
 // and blocks until f returns or calls t.Parallel to become a parallel test.
 // Run reports whether f succeeded (or at least did not fail before calling t.Parallel).
@@ -1463,6 +1486,15 @@ func (t *T) Run(name string, f func(t *T)) bool {
        if !ok || shouldFailFast() {
                return true
        }
+
+       if loc, ok := t.cases[name]; ok {
+               fo := f
+               f = func(t *T) {
+                       t.Helper()
+                       t.Logf("case at %s", loc)
+                       fo(t)
+               }
+       }
        // Record the stack trace at the point of this call so that if the subtest
        // function - which runs in a separate stack - is marked as a helper, we can
        // continue walking the stack into the parent test.

package go52751

import (
    "testing"
)

func TestFileLine(t *testing.T) {
    tests := []struct {
        name  string
        input string
    }{{
        name:  t.Case("Foo"),
        input: "x",
    }, {
        name:  t.Case("Bar"),
        input: "",
    },
    }
    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            if tt.input == "" {
                t.Fatalf("input error")
            }
        })
    }
}

--- FAIL: TestFileLine (0.00s)
    --- FAIL: TestFileLine/Bar (0.00s)
        case_test.go:20: case at /tmp/go52751/case_test.go:15
        case_test.go:22: input error

[rsc]

@dsnet, any thoughts on https://github.com/golang/go/issues/52751#issuecomment-1124087356 ?

[dsnet]

In regards to https://github.com/golang/go/issues/52751#issuecomment-1124087356, it seems odd if the test name is annotated with file:line information if we can directly use the test name and file:line information together. There's a discrepancy between how creation and usage operates.

If usage is segregated, then creation should also be segregated:

  tests := struct {
      name string
+     location testing.SourceLocation
      input T
      ...
  } {{
      name: "Foo",
+     location: testing.FileLine(),
      ...,
  }, {
      name: "Bar",
+     location: testing.FileLine(), // my_test.go:321
      ...,
  }
  ... // maybe dozens or hundreds more cases
  }
  for _, tt := range tests {
      t.Run(tt.name, func(t *testing.T) {
+         t.Annotate(tt.location)
          got, err := fizz(tt.input)
          if err != nil {
              t.Fatalf("fizz error: %v", err) // my_test.go:1234
          }
      })
  }

This approach is more flexible as you can imagine using this for more purposes than just annotating test case names. However, it is a little more typing than my original proposal in https://github.com/golang/go/issues/52751#issue-1228470296. I would be okay with something like this API.

---

In response to https://github.com/golang/go/issues/52751#issuecomment-1140119744, one significant detriment to func (t *T) Case(name string) string is that it assumes that we have a testing.T on hand when crafting the test cases. I have some tests that share a global testcases table. There is no single testing.T around at the time that the table was constructed.

[rsc]

@dsnet, you jumped to testing.SourceLocation, which I didn't really have in mind. I think testing.Name returning something that has a name and a file:line is fine. But there are other tests reading test data files that might also want to set the file:line, and I was suggesting that they could use that mechanism too if it were split out. I don't think we need a SourceLocation type though.

So only the t.Annotate would be added in the diff, not all the testing.FileLine() calls.

[rsc]

Ping @dsnet

[dnephin]

I tried out a version of this in one of my test cases, and it seems that at least GoLand IDE does not highlight multiple file:line on the same line. The first instance of file:line on each output line is a hyperlink to the file in the IDE, but subsequent ones are just plain text.

I'm less familiar with other environments, but I expect even in environments that don't provide hyperlinks to the source it wont be obvious to someone running the tests what each of the two file:line are supposed to reference.

The ability to add file:line annotations to test failures.

I agree this is what is missing, and I think this would be more valuable if it worked similar to https://github.com/golang/go/issues/52751#issuecomment-1124477993. Instead of a second file:line number on the line, the location would replace the one that came from t.Log.

As multiple people have mentioned on this thread, capturing the file and line number is pretty easy to do, and could be done in different ways depending on the test.

If testing.T had a function that logged without any decoration a caller could pass in the decoration themselves:

location := fileLine()
...
t.Println(fmt.Sprintf("%v: test case definition", location))

Where Println would be like Log without c.deocrate:

func (c *common) Println(args ...any) {
    ... // handle c.done, and c.chatty branches
    c.output = append(c.output, fmt.Sprint(args...))
}

The output from the original proposal would look something like this:

--- FAIL: Test/Bar (0.00s)
    my_test.go:321: test case definition
    my_test.go:1234: fizz error: the world blew up

Edit: it is possible to use fmt.Println to remove the decoration, but that output appears in a different place from t.Log output in the standard (non-verbose) go test format.

Edit 2: it seems as of go1.20 the stdout is now interleaved with t.Log output, but is not indented the same way.

[dsnet]

it seems that at least GoLand IDE does not highlight multiple

It's unfortunate that GoLand doesn't adequately hyperlink file:line: strings. I use VSCode, which does a decent job at it. Regardless of IDE support, the testing package already outputs file:line: annotations for test error and log messages and so doing more of it should not be a surprise.

If testing.T had a function that logged without any decoration a caller could pass in the decoration themselves:

I'm starting to lean towards this approach. I haven't quite come to terms with how much this should be in testing versus how much this should be a helper in runtime.

(I'm going to go on a tangent now regarding logging but it is related)

The other day I was dealing with log functions and composing them together. In my use case, I had a log.Logger.Printf and also another logf-like function that wrote to a different sink. I wanted to compose these two together so a single logf would write to both:

func multiLogf(logfs ...func(string, ...any)) func(string, ...any) {
    return func(f string, a ...any) {
        for _, logf := range logfs {
            logf(f, a...)
        }
    }
}

The problem with this is that log.Lshortfile does not compose well and now always records the callsite in my multiLogf helper function, making it fairly useless.

One way to address this is to have logf-like functions never record file:line and have the caller do it manually. However, we would want this to be easy to do. Unfortunately, the runtime.Caller API is not particularly friendly because it returns multiple arguments, making it difficult to use in a Go expression.

Imagine there was a:

package runtime

// FileLine returns the file:line of the caller.
// The file is only the base file name.
func FileLine() string

then I could have done:

logf("%s: payload too large for %v", runtime.FileLine(), id)

Similarly, we can accomplish what this proposal was originally about with:

  tests := struct {
+     where string
      name  string
      input T
      ...
  } {{
+     where: runtime.FileLine(),
      name:  "Foo",
      ...,
  }, {
+     where: runtime.FileLine(),
      name:  "Bar",
      ...,
  }
  ... // maybe dozens or hundreds more cases
  }
  for _, tt := range tests {
      t.Run(tt.name, func(t *testing.T) {
+         t.Logf("%s: test case declaration", tc.where)
          got, err := fizz(tt.input)
          if err != nil {
              t.Fatalf("fizz error: %v", err)
          }
      })
  }

Having a helper function in runtime makes grabbing the "file:line" useful for more applications than just testing. However, I'd be okay with a helper also in testing that combined creation of a name and also where it was called from (essentially point 1 in https://github.com/golang/go/issues/52751#issuecomment-1124087356).

My concern with point 2 in https://github.com/golang/go/issues/52751#issuecomment-1124087356 is that I'm uncertain how this information should be presented in the terminal UI and also what happens when t.AddFileLine(tt.name.FileLine()) is called multiple times. Making it something we annotate manually with testing.TB.Logf allows for more experimentation.

That said, I'd be okay even with something like testing.TB.AddFileLine if we can nail down the semantics. Alternatively, a testing.TB.Println method that doesn't log the line like what @dnephin suggested could work. The downside with that approach is that some users will call testing.TB.Println when they really should be calling testing.TB.Log.

[rsc]

I don't understand why we'd add a Println etc that does not log the source file line. If the caller is going to add their own explicitly, there's no harm in having two file:line in the output. I have tests that do that (for example cmd/go TestScript) and it works just fine. You get the source line of the log message as well as the source line of the corresponding data and can click on either one depending on what you are looking for.

Suppose we had

func (t *T) Source(file string, line int) 

which adds that file:line to the list of file lines that get printed. For example if you do

t.Source("mydata.txt", 2)
t.Fatalf("oops")

the message would look like

mydata.txt:2: my_test.go:123: oops

That seems generally useful, and I would probably use it in TestScript instead of the manual prints that I have today.

Then the remaining question is how to make it easy to get a file,line to pass to Source for a table-driven test.

Suppose we defined

// A Marker records a name at a given Go source file position.
type Marker struct {
    Name string
    File string
    Line int
}

func (m Marker) Source() (string, int) { return m.File, m.Line }

// Mark returns a Marker with the given name
// that also records the Go source file position where it appears.
func Mark(name string) Marker

Then we could do something like:

var squareTests = []struct{
    m Marker
    in int
    out int
}{
    {testing.Mark("zero"), 0, 0},
    {testing.Mark("negative"), -2, 4},
    {testing.Mark("positive"), 3, 9},
    {testing.Mark("huge"), 1000, 1000_000},
}

for _, tt := range squareTests {
    t.Run(tt.m.Name, func(t *testing.T) {
        t.Source(tt.m.Source())
        if out := tt.in*tt.in; out != tt.out {
            t.Errorf("%d*%d = %d, want %d", tt.in, tt.in, out, tt.out)
        }
    })
}

It seems to me that this separation provides two generally useful concepts that work well apart as well as together, instead of tying them together into one specialized concept.

Thoughts?

[dsnet]

Overall, SGTM.

Some questions about semantics:

• What happens when Source is called multiple times? Do they stack up?
• When you call testing.T.Run, does it preserve the source called on the parent T?
• Does Source affect the output of a panic? If a test panics, I would expect the source to be printed somewhere since that's relevant information.

[rsc]

Let's try to answer those questions:

1. Calling Source multiple times stacks.
2. Calling Run starts the new T with the Source info from the old T, and then the new one can stack additional ones.
3. We should probably print a log message on all panics, like t.Log("PANIC:", v), and then that will show the source lines for us.

Thoughts?

[rsc]

It seems like people are mostly happy with https://github.com/golang/go/issues/52751#issuecomment-1218264156? Does anyone want to try to implement it and see if it makes any sense in the implementation?

[rsc]

On hold for an implementation to sanity check this idea.

[rsc]

Placed on hold. — rsc for the proposal review group

[gopherbot]

Change https://go.dev/cl/444195 mentions this issue: testing: add TB.Source to support user-specified source files

[dsnet]

I sent out CL 444195 as a prototype implementation.

Example usage:

func Test(t *testing.T) {
    tests := []struct {
        testing.Marker
        in   int
        want string
    }{
        {testing.Mark("Foo"), 1, "1"},
        {testing.Mark("Bar"), 2, "2"},
        {testing.Mark("Baz"), 3, "3"},
        {testing.Mark("Giz"), 4, "4"},
        {testing.Mark("Daz"), 5, "5"},
    }
    for _, tt := range tests {
        t.Run(tt.Name, func(t *testing.T) {
            t.Source(tt.Source())
            got := strconv.Itoa(2 * (tt.in / 2)) // faulty conversion
            if got != tt.want {
                t.Errorf("Itoa(%d) = %v, want %v", tt.in, got, tt.want)
            }
        })
    }
}

Example output:

--- FAIL: Test (0.00s)
    --- FAIL: Test/Foo (0.00s)
        main_test.go:16: main_test.go:27: Itoa(1) = 0, want 1
    --- FAIL: Test/Baz (0.00s)
        main_test.go:18: main_test.go:27: Itoa(3) = 2, want 3
    --- FAIL: Test/Daz (0.00s)
        main_test.go:20: main_test.go:27: Itoa(5) = 4, want 5

where lines 16, 18, 20 are where the test case was declared, and line 27 is where a test error was reported.

Overall, I generally like the API except for the following:

• The t.Source(tt.Source()) call stutters. I don't have any great ideas. Having TB.Source take in a testing.Marker seems wrong.

• The API can be easy to hold wrong.

  Consider the following modification of the test above:

    for _, tt := range tests {
  +   t.Source(tt.Source())
      t.Run(tt.Name, func(t *testing.T) {
  -       t.Source(tt.Source())
          got := strconv.Itoa(2 * (tt.in / 2)) // faulty conversion
          if got != tt.want {
              t.Errorf("Itoa(%d) = %v, want %v", tt.in, got, tt.want)
          }
      })
    }

  where the call to t.Source(tt.Source()) was simply flipped, which would result in an output of:

  --- FAIL: Test (0.00s)
      --- FAIL: Test/Foo (0.00s)
          main_test.go:16: main_test.go:27: Itoa(1) = 0, want 1
      --- FAIL: Test/Baz (0.00s)
          main_test.go:16: main_test.go:17: main_test.go:18: main_test.go:27: Itoa(3) = 2, want 3
      --- FAIL: Test/Daz (0.00s)
          main_test.go:16: main_test.go:17: main_test.go:18: main_test.go:19: main_test.go:20: main_test.go:27: Itoa(5) = 4, want 5

  The multiple calls to Source is stacked up per the semantics suggested in https://github.com/golang/go/issues/52751#issuecomment-1233240387.

  One way to address this is to have Source only remember the last provided source. This would generally work well for any non-parallel tests (which are the most common).

Alternatively, the originally proposed API of have a new Run method would solve both issues:

func (*T) RunMarker(Marker, func(*T))

since each instantiation of T may have at most one user-specified source location associated with it. Furthermore, the source location is immutable once associated, which avoids any strange business with it being unintentionally stacked up or replaced. This would simplify the implementation since we can avoid grabbing locks for all the AP calls.

[dsnet]

I think my ideal API for this continues to be something like:

type Label struct {
    Name string
    File string
    Line int
}
func Mark(name string) Label

func (*T) RunLabel(Label, func(*T))
func (*B) RunLabel(Label, func(*B))

As a bikeshed, renaming Marker to Label seems clearer to me as the testing.Marker type by itself doesn't really tell me anything. An alternative verb for Mark could be Annotate.

[AlexanderYastrebov]

As a bikeshed, renaming Marker to Label seems clearer to me

Why not testing.Case as it is, well, a test case?

[AlexanderYastrebov]

I think my ideal API for this continues to be something like: func (T) RunLabel(Label, func(T)) func (B) RunLabel(Label, func(B))

https://github.com/golang/go/issues/52751#issuecomment-1140093481

you would need two different Run methods since the method must taking in either a func(testing.T) or a func(testing.B). It's not clear that it's a cleaner API than what was originally proposed.

It could be either two methods on one type:

type TC struct {
    Name string
    File string
    Line int
}

func Case(name string) TC

func (tc TC) Test(t *T, f func(t *T)) bool
func (tc TC) Benchmark(t *B, f func(t *B)) bool

or two case types:

func Case(name string) TC
func Benchmark(name string) TBC

func (tc TC) Run(t *T, f func(t *T)) bool
func (tc TBC) Run(t *B, f func(t *B)) bool

[dsnet]

Moving out of hold per https://github.com/golang/go/issues/52751#issuecomment-1268744199 since a prototype implementation exists.

[flowchartsman]

As some of the other posters mentioned t.Helper() can already be really handy to "flatten" table-driven tests. Is there anything about this proposal that is preferable to an idiom like this?

func Test(t *testing.T) {
    type ItoaTest struct {
        name string
        item int
        want string
    }
    run := func(test ItoaTest) {
        t.Helper()
        got := strconv.Itoa(2 * (test.item / 2))
        if got != test.want {
            t.Errorf("%s - Itoa(%d) = %v, want %v", test.name, test.item, got, test.want)
        }
    }
    run(ItoaTest{"First", 1, "1"})
    run(ItoaTest{"Second", 2, "2"})
    run(ItoaTest{"Third", 3, "3"})
    run(ItoaTest{"Fourth", 4, "4"})
    run(ItoaTest{"Fifth", 5, "5"})
}

=== RUN   Test
    prog.go:23: First - Itoa(1) = 0, want 1
    prog.go:25: Third - Itoa(3) = 2, want 3
    prog.go:27: Fifth - Itoa(5) = 4, want 5
--- FAIL: Test (0.00s)
FAIL

[dsnet]

@flowchartsman that doesn't help the situation where run is a sufficiently complex test function. You now lose information about where it failed. You forced to select between either 1) where in the test code it failed, or 2) what test input led to this failure. Unfortunately, you can't have both.

[flowchartsman]

Is this something where the clever use of type parameters can help?

TableTest[squareTest](t).
        Case("zero", squareTest{0, 0}).
        Case("negative", squareTest{-2, 4}).
        Case("positive", squareTest{3, 9}).
        Case("huge", squareTest{1000, 1000_000}).
        Case("fail", squareTest{1, 0}).
        Run(func(t *testing.T, c squareTest) {
            if res := c.in * c.in; res != c.out {
                t.Errorf("%d*%d = %d, want %d", c.in, c.in, res, c.out)
            }
        })

=== RUN   TestSquare
=== RUN   TestSquare/[0](zero)[/tmp/sandbox4128893806/prog.go:48]
=== RUN   TestSquare/[1](negative)[/tmp/sandbox4128893806/prog.go:49]
=== RUN   TestSquare/[2](positive)[/tmp/sandbox4128893806/prog.go:50]
=== RUN   TestSquare/[3](huge)[/tmp/sandbox4128893806/prog.go:51]
=== RUN   TestSquare/[4](fail)[/tmp/sandbox4128893806/prog.go:52]
    prog.go:55: 1*1 = 1, want 0
--- FAIL: TestSquare (0.00s)
    --- PASS: TestSquare/[0](zero)[/tmp/sandbox4128893806/prog.go:48] (0.00s)
    --- PASS: TestSquare/[1](negative)[/tmp/sandbox4128893806/prog.go:49] (0.00s)
    --- PASS: TestSquare/[2](positive)[/tmp/sandbox4128893806/prog.go:50] (0.00s)
    --- PASS: TestSquare/[3](huge)[/tmp/sandbox4128893806/prog.go:51] (0.00s)
    --- FAIL: TestSquare/[4](fail)[/tmp/sandbox4128893806/prog.go:52] (0.00s)
FAIL

https://go.dev/play/p/bWVlsiS1jZ6

Though I suspect the fluent-style interface is a turn-off, there might be some extra tricks you can do.

[flowchartsman]

Or, perhaps as a generic slice, keeping that table-driven feel:

    TableTest[squareTest]{
        Case("zero", squareTest{0, 0}),
        Case("negative", squareTest{-2, 4}),
        Case("positive", squareTest{3, 9}),
        Case("huge", squareTest{1000, 1000_000}),
        Case("fail", squareTest{1, 0}),
    }.Run(t, func(t *testing.T, c squareTest) {
        if res := c.in * c.in; res != c.out {
            t.Errorf("%d*%d = %d, want %d", c.in, c.in, res, c.out)
        }
    })

https://go.dev/play/p/1FWoYO8zE3z

[AlexanderYastrebov]

@flowchartsman Embedding location into test name in this form makes it hard to run individual test via -run flag (https://pkg.go.dev/cmd/go#hdr-Testing_flags).

[AlexanderYastrebov]

@dsnet

In response to #52751 (comment), one significant detriment to func (t *T) Case(name string) string is that it assumes that we have a testing.T on hand when crafting the test cases. I have some tests that share a global testcases table. There is no single testing.T around at the time that the table was constructed.

I think *testing.T is available much more often than not. The func (t *T) Case(name string) string approach is backward compatible, requires no API change (only an extension) and requires no effort from user other than wrapping test case name. So maybe it could be considered by putting static testcase tables out of scope for this feature.

Maybe with some careful effort cases map[string]string could be even made global to also support static testcase tables via testing.Case instead of t.Case.

[flowchartsman]

@flowchartsman Embedding location into test name in this form makes it hard to run individual test via -run flag (https://pkg.go.dev/cmd/go#hdr-Testing_flags).

I agree. I should have been more clear, I am attempting to demonstrate a POC for general execution, however I might also argue that running individual tests out of a table-driven test is something that only certain IDEs do to my knowledge, and it might be out of scope. Still, there might be a solution if you're clever enough. If there's actual interest, I'd be willing to implement a prototype in testing itself.

[rsc]

Here is the API from CL 444195:

type Marker struct {
    Name string
    File string
    Line int
}

func Mark(name string) Marker
func (m Marker) Source() (file string, line int) 

func (t *T) Source(file string, line int)
func (b *B) Source(file string, line int)
(and add Source to TB)

And it is used as

    tests := []struct{
        testing.Marker
        ...
    }{
        {testing.Mark("Foo"), ...},   <<<<<
        {testing.Mark("Bar"), ...},
        ... // maybe dozens or hundreds more cases
    }
    for _, tt := range tests {
        t.Run(tt.Name, func(t *testing.T) {
            t.Source(tt.Source())   <<<<<
            ... // perform the test
        })
    }

The t.Source line makes the failure logs print the additional file:line at the start of the messages:

    --- FAIL: Foo (0.00s)
        source_test.go:33: source_test.go:46: Itoa(1) = 0, want 1
    --- FAIL: Baz (0.00s)
        source_test.go:35: source_test.go:46: Itoa(3) = 2, want 3
    --- FAIL: Daz (0.00s)
        source_test.go:37: source_test.go:46: Itoa(5) = 4, want 5

This does require having a name in the test struct, which not all table-driven tests have. Another option would be to separate them out so that Mark takes no arguments and Marker has no Name inside it. Then you'd use

    tests := []struct{
        name string
        mark testing.Marker
        ...
    }{
        {"Foo", testing.Mark(), ...},   <<<<<
        {"Bar", testing.Mark(), ...},
        ... // maybe dozens or hundreds more cases
    }
    for _, tt := range tests {
        t.Run(tt.Name, func(t *testing.T) {
            t.Source(tt.mark.Source())   <<<<<
            ... // perform the test
        })
    }

And at that point you could make Source take a Marker directly:

        t.Source(tt.mark)   <<<<<

and at that point maybe the type name be Source instead of Marker:

tests := []struct{
    name string
    src testing.Source
    ...
}{
    {"Foo", testing.Source(), ...},   <<<<<
    {"Bar", testing.Source(), ...},
    ... // maybe dozens or hundreds more cases
}
for _, tt := range tests {
    t.Run(tt.Name, func(t *testing.T) {
        t.Source(tt.src)   <<<<<
        ... // perform the test
    })
}

The meaning of t.Source is still a bit hard to understand if you don't read the docs. And Source can't be both a method and a type.

What about

type Pos struct {File string; Line int}
testing.Mark() Pos
t.AddPos(tt.pos)

?

[dnephin]

It sounds like the consensus so far is to include the extra file:line output on the same line as the output from all of the subsequent calls to t.Log, t.Fatal, t.Error, etc.

If I understand correctly, that means a test with multiple calls to t.Log, t.Error, etc would produce output like this:

--- FAIL: Foo (0.00s)
    source_test.go:33: source_test.go:46: first output
    source_test.go:33: source_test.go:55: second output
    source_test.go:33: source_test.go:72: third output

I would like to make one final attempt to argue for separate lines of output:

--- FAIL: Foo (0.00s)
    source_test.go:33: test case definition
    source_test.go:46: first output
    source_test.go:55: second output
    source_test.go:72: third output

My previous argument for this approach was that it's easier to understand which of these file:line markers refers to the test case definition, and which one of them refers to lines in the the test logic. The author of the test is able to provide that context to the person reading the test failure.

With the acceptance of #37708, I think multiple file:line markers on the same line become much harder for software to parse (which I believe is the primary motivation for #37708). The first file:line (or fullpath:line) in a line of output is unambiguous. The line can be strings.Cut at the first occurrence of :, and then again at the last occurrence of : to find the file path and line number.

As soon as multiple file:line numbers are introduced it may not be possible to unambiguously parse those markers. The output after the first marker could be output that happens to include a colon followed by numbers. File paths can include spaces and colons, so it's not clear where the file:line markers stop, and where the output begins. How would programs parse these markers when -test.fullpath is introduced?

---

Edit: the API suggestion below does not attribute the line to the right subtest. It would require at least another method or type.

I believe the API to support separate lines would also be smaller. Instead of needing 3 new symbols (a type, a function, and a method on testing.T), a single method could provide the full functionality:

testing.T.Mark(args ...any)

args is the same as in t.Skip, t.Fatal, etc. Optional output that will be printed after the file:line marker.

One disadvantage to this API is that it would not provide anything new for other sources of file:line markers (testscript failures). They would have to use the same mechanism that they use today, which I guess it to print the file:line marker using t.Log.

[neild]

    --- FAIL: Foo (0.00s)
        source_test.go:33: source_test.go:46: Itoa(1) = 0, want 1

This seems a bit unfortunate, since every line of output for a test will have the same prefix. Also, as @dnephin points out, I don't believe editors generally recognize multiple filename:line prefixes on the same line.

What if the marker was associated with the test line?

    --- FAIL: Foo [source_test.go:33] (0.00s)
        source_test.go:46: Itoa(1) = 0, want 1

Also, bikeshedding on the API a bit, what if we passed the source information to t.Run via the test name?

for _, test := range []struct{
  name string
  // ...
}{{
  name: testing.Case("Foo"),
  // ...
}}{
  t.Run(test.name, func(t *testing.T) {
    // ...
  })
}

The testing.Case function returns a string along the lines of "Foo [source_test.go:46]". T.Run recognizes a bracketed string at the end of the test name as source information, and presents it appropriately. This is technically backwards incompatible if someone has a test case that contains a bracketed string suffix, but we could choose a form that is unlikely to collide with existing test names. Or we could just say that the source information is part of the test name, which has the advantage of making it easy to specify a test by location: go test . -run=source_test.go:33:

    --- FAIL: Foo_[source_test.go:33] (0.00s)
        source_test.go:46: Itoa(1) = 0, want 1

This requires no changes to test code other than to use testing.Case(name) to create test names.

Making the source information actually part of the test name does have the unfortunate property that the name is not stable over time, so perhaps that isn't a good idea. On the other hand, systems that care about name stability can strip off the bracketed suffix, so perhaps it wouldn't be a problem.

[AlexanderYastrebov]

What if the marker was associated with the test line?

I think there was an idea to support multiple source locations for a single test case.

T.Run recognizes a bracketed string at the end of the test name as source information, and presents it appropriately.

Or testing.Case("Foo") can record test name to source mapping in a global map

[dsnet]

This is technically backwards incompatible if someone has a test case that contains a bracketed string suffix

I thought about what you're suggesting and rejected it myself for the technically backwards incompatibility argument. That said, I am personally in support of this approach for it's simplicity.

If we can't change the behavior of testing.T.Run, the closest compatible API would be something like https://github.com/golang/go/issues/52751#issuecomment-1284459876.

Making the source information actually part of the test name does have the unfortunate property that the name is not stable over time, so perhaps that isn't a good idea. On the other hand, systems that care about name stability can strip off the bracketed suffix, so perhaps it wouldn't be a problem.

So long as the name used to match the argument given to go test -run=$MATCH does not consider the filename, I think it should be relatively inconsequential.

With the recent adoption of #37708, we'll want to consider how that plays into this proposal. For @neild's proposal, the path passed to testing.T.Run should probably include the full path, and it is the reporter logic's job to decide whether to show the source file only (the default) or the full path (if -fullpath is specified).

Or testing.Case("Foo") can record test name to source mapping in a global map

Interesting idea, but how would you deal with conflicts? It is fairly conceivable that two cases in different contexts might both be named "Foo".

[AlexanderYastrebov]

Interesting idea, but how would you deal with conflicts? It is fairly conceivable that two cases in different contexts might both be named "Foo".

Probably would need to count duplicates and return numbered names Foo, Foo#1, Foo#2, etc (like (m *matcher) unique(...) does but that one works in a context of the parent test)

[dsnet]

I don't think that works well for cases that would not normally need deduplication. For example:

var fooCases = []struct { ... } {
    {testing.Case("FizzBuzz"), ...},
    ...
}

func TestBar(t *testing.T) {
    ... // uses fooCases
}

var barCases = []struct { ... } {
    {testing.Case("FizzBuzz"), ...},
    ...
}

func TestBar(t *testing.T) {
    ... // uses barCases
}

Both occurrences of FizzBuzz occur in different namespaces (i.e., Foo and Bar), but the invocation of testing.Case has no idea that it would never naturally conflict since the full name would be Foo/FizzBuzz or Bar/FizzBuzz. As a result it would unnecessarily need to deconflict the names as Foo/FizzBuzz and Bar/FizzBuzz#1, which is highly unfortunate.

[dnephin]

type Pos struct {File string; Line int}
testing.Mark() Pos
t.AddPos(tt.pos)

Instead of t.AddPos, could the file:line be printed using a new %l (line) or %m (marker) verb for the existing t.Log, t.Logf, t.Error, etc methods? The only differences from %v would be:

1. it instructs t.Log to replace the default file:line added by testing with the testing.Pos
2. it would use -fullpath from #37708 to change the output from the default filename to a full path

That would allow the test author to replace the default file:line marker if they choose, but should also allow for multiple file:line in the same log message.

type testCase struct {
    name string
    pos testing.Pos
    ...
}
...

// t.Log, t.Error, t.Fatal with a testing.Pos at args[0] replaces the default file:line marker
t.Log(tc.pos, "test case definition")
// source.go:123: test case definition

// testing.Pos at args[1:] would continue to work
t.Log("failed at", tc.pos)
// source.go:444: failed at source.go:123

// t.Logf, t.Errorf, t.Fatalf use the new verb to replace the default file:line marker
t.Logf("%l: test case definition", tc.pos)
// source.go:123: test case definition

// the position can also be used with the default file:line marker
t.Fatalf("%v: got: ..., want: ...", tc.pos, got, want)
// source.go:444: source.go:123: got: ..., want: ...

testing.Pos seems to be quite similar to https://pkg.go.dev/go/token#Position. I guess they are roughly storing the same thing ("an arbitrary source position"), and the Position.String method seems to print the right format. I imagine there may be good reasons to not re-use that type, but I thought the similarity was interesting.

This approach would reduce the API to a single new testing.Mark. As @dsnet suggested earlier, if that function is a helper around runtime.Caller, maybe testing.Mark could be runtime.FileLine() Position, and the only changes to testing would be handling the Position and format verb.

[neild]

Proposal:

When a test name passed to t.Run contains a newline, the test name is everything up to the newline and the remainder is printed at the start of the test log.

func Test(t *testing.T) {
  t.Run("Foo\nmore information", func(t *testing.T) {
    t.Fatal("oops")
  }
}

--- FAIL: Test (0.00s)
    --- Fail: Test/Foo (0.00s)
        more information
        x_test.go:1000: oops

(To bikeshed: What if there are multiple newlines?)

The new function testing.Case(name) returns name + "\n" + filename + ":" + lineno.

func Test(t *testing.T) {
  for _, test := range []struct{
    name string
  }{{
    name: testing.Case("Foo"),
  }}{
    t.Run(test.name, func(t *testing.T) {
      t.Fatal("oops")
    }
  }
}

--- FAIL: Test (0.00s)
    --- Fail: Test/Foo (0.00s)
        x_test.go:1000
        x_test.go:1003: oops

It's unlikely that anyone is currently defining a test name containing a newline, since I'm pretty sure test output breaks if you do. This provides a mechanism to add other per-case information than filename and line number, and doesn't require any changes to existing test output parsers. Updating existing tests is just changing "name" to testing.Case("name").

[dnephin]

It's unlikely that anyone is currently defining a test name containing a newline, since I'm pretty sure test output breaks if you do.

I believe the newline is converted to a _ in the output (like other whitespace characters): https://go.dev/play/p/Ic4FH_ZKkzW, so it's possible somewhere there are test names with newlines, but most likely it's rare.

I like the approach of associating the marker with a name. Earlier suggestions like https://github.com/golang/go/issues/52751#issuecomment-1284459876 seemed like a nice way to make that work.

[rsc]

It seemed like we were at a reasonable point with

type Pos struct {File string; Line int}
testing.Mark() Pos
t.AddPos(testCase.pos)

There was a concern raised about multiple file:line: on a given line, but that should be plenty legible, and at least the tools I use have no problem with linking to either one. If others know of specifically broken environments that don't support that, let's look at concrete cases.

The discussion moved over to trying to smuggle information into the t.Run argument, but that's a pretty major change, and a confusing one. We should try to use data types and data structures instead of appending things to the t.Run argument.

Other than the question of how to print two file:line associated with a message, are there other concerns remaining about this proposal?

[dsnet]

There was a concern raised about multiple file:line: on a given line, but that should be plenty legible

When combined with the new -fullpath flag in https://github.com/golang/go/issues/37708, I don't see how this will remain legible. I'd argue there's only enough space for a single full path to be comfortable printed per line.

I also commented in https://github.com/golang/go/issues/52751#issuecomment-1284451116 that the API was easy to accidentally misuse with a one-line mistake like:

  for _, tt := range tests {
+   t.Source(tt.Source())
    t.Run(tt.Name, func(t *testing.T) {
-       t.Source(tt.Source())
        ...
    })
  }

which would drastically change the way test output was formatted. The added line would (unintentionally) cause multiple source positions to be stacked up, while the removed line would (correctly) only print one source position per t.Run invocation.

In https://github.com/golang/go/issues/52751#issuecomment-1284459876, I concluded that I still believed the best solution was still a new RunLabel method, which solves both issues:

1. it ensures that at most one path may be associated with a test name, avoiding unreadable stack-up of many source paths (especially if -fullpath is specified)
2. Since the source information is provided at the same time to Run, there is no possibility of accidentally registering at the wrong time (i.e., too early or too late).

While I advocated for a new RunLabel API, @neild suggested hijacking the existing Run method and giving special semantics to certain strings to be treated as a file path. Personally, I'd be happy with either a new RunLabel method or modifying Run to detect file paths.

[rsc]

Mixing the source into Run seems like it will end up being a non-orthogonal issue and lead to more, and then we'll need all the possible combinations of arguments to Run. A helper on testing.T seems clearer, and mostly separates it from Run.

As for the mistake of calling t.Source in the outer loop, maybe Source should only be setting a single source instead of stacking up? Then it's not tied to Run. It would probably be t.SetPos then, and the test log messages would print the Go source line as well as the position set by SetPos.

[dnephin]

If others know of specifically broken environments that don't support [multiple file:line: on a given line], let's look at concrete cases.

GoLand (the JetBrains IDE for Go) does not support this. Here's an example screenshot from the terminal window in GoLand:

[dsnet]

we'll need all the possible combinations of arguments to Run

Is this perhaps instead an argument that there's a variation of Run with variadic optional arguments? 🤔

maybe Source should only be setting a single source instead of stacking up

Quite possibly. I would support this semantic more than the stacking up semantic, but it could interact poorly with usages of T.Parallel.