Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testing: loses log messages made in a goroutine after test has completed #30389

Open
ianlancetaylor opened this issue Feb 25, 2019 · 1 comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

The testing package silently drops t.Log messages that are made by a goroutine after the test has completed. For example, the following test passes and logs nothing.

This should be fixed by extending the code added in 1.12 that panics if a goroutine logs after a test has completed to not silently discard the log message. The silent discarding is occurring in the current code because every test runs in a parent context, and the output accumulated by the parent context is discarded.

package main

import (
	"testing"
	"time"
)

func TestLateLog(t *testing.T) {
	c := make(chan bool)
	go func() {
		<-c
		time.Sleep(time.Millisecond)
		t.Log("log after test")
	}()
	close(c)
}
@ianlancetaylor ianlancetaylor added the NeedsFix The path to resolution is known, but the work has not been done. label Feb 25, 2019
@ianlancetaylor ianlancetaylor added this to the Go1.13 milestone Feb 25, 2019
@rhnvrm
Copy link
Contributor

rhnvrm commented Feb 27, 2019

Hey,

I tried investigating and wrote a test and modifying various places in testing.go like the log and logDepth function in my fork with debug statements (fmt.Fprintf(os.Stdout, "%s", string(s))) but the log never ran.

I might be naively making a mistake here but I guess after the TestLateLog function exits t.output or t.parent output is never changed and flush output never has anything to print.

func TestLateLogWrapper(t *testing.T) {
	testenv.MustHaveGoBuild(t)
	tempDir, err := ioutil.TempDir("", "issue-30389")
	if err != nil {
		t.Fatalf("Failed to create temp directory: %v", err)
	}
	defer os.RemoveAll(tempDir)
	code := `
package main

import (
	"testing"
	"time"
	"fmt"
)

func TestLateLog(t *testing.T) {
	c := make(chan bool)
	go func(t *testing.T) {
		<-c
		time.Sleep(10*time.Millisecond)
		fmt.Printf("-> %v\n", t)
		t.Log("log after test")
	}(t)
	close(c)
}
`
	destFile := filepath.Join(tempDir, "issue30389_test.go")
	if err := ioutil.WriteFile(destFile, []byte(code), 0644); err != nil {
		t.Fatalf("Failed to save test code: %v", err)
	}
	got, err := exec.Command(testenv.GoToolPath(t), "test", destFile).CombinedOutput()
	if err == nil {
		t.Fatal("The command unexpectedly passed")
	}
	want := []byte(`xxx`)
	if !bytes.HasPrefix(got, want) {
		t.Errorf("Did not print previous output\nGot:\n%s\n\nWant prefix:\n%s", got, want)
	}
}

Another thing I had a doubt about was, if we are to add a panic before the following to notify users that this information will be lost (or even for debugging if we reach there for this specific case)
parent.output = append(parent.output, parent.decorate(s, depth+1)...)
is there a way to detect that it was a test without a tRunner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants