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

Jump to bottom

testing: Helper line number has changed in 1.16 #44887

Closed
ianlancetaylor opened this issue Mar 9, 2021 · 5 comments
Closed

testing: Helper line number has changed in 1.16 #44887

ianlancetaylor opened this issue Mar 9, 2021 · 5 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone
Go1.17

Comments

@ianlancetaylor
Copy link
Contributor

This is the test program.

package main_test

import (
	"testing"
)

func TestHelper(t *testing.T) {
	helperA(t)
}

func helperA(t *testing.T) {
	t.Helper()

	t.Run("subtest", func(t *testing.T) {
		t.Helper()
		t.Fatal("fatal error message")
	})
}

When run using go test foo_test.go with Go 1.14 or Go 1.15, this program prints

--- FAIL: TestHelper (0.00s)
    --- FAIL: TestHelper/subtest (0.00s)
        foo_test.go:8: fatal error message
FAIL
FAIL	command-line-arguments	0.009s
FAIL

When run with Go 1.16 it prints

--- FAIL: TestHelper (0.00s)
    --- FAIL: TestHelper/subtest (0.00s)
        foo_test.go:14: fatal error message
FAIL
FAIL	command-line-arguments	0.007s
FAIL

The line number has changed from 8 to 14. Line number 14 is in a function that called t.Helper, so line 8 seems correct and line 14 seems wrong.

This appears to be due to https://golang.org/cl/231717. If I revert that CL, I see line number 8.

CC @bboreham
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 9, 2021
@ianlancetaylor ianlancetaylor added this to the Go1.17 milestone Mar 9, 2021
@ianlancetaylor
Copy link
Contributor Author

@gopherbot Please open backport to 1.16

In some cases t.Helper seems to be ineffective. It's not critical but it's definitely annoying. We should fix it.

@gopherbot
Copy link

Backport issue(s) opened: #44888 (for 1.16).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

@gopherbot gopherbot mentioned this issue Mar 9, 2021
Closed
@dnephin
Copy link
Contributor

dnephin commented Mar 15, 2021

I just ran into this problem. I have been using a function runCase (see below) to make it easier to work with very large table tests. When a test spans thousands of lines it can be difficult to find the data for a specific failure. This runCase pattern has made it much easier to debug failing tests, and it no longer works due to this regression.

I notice this issue has been pushed out a couple patch releases. Please let me know if I can be of any help in restoring the old behaviour or testing a fix.

Example use case:

func runCase(t *testing.T, name string, fn func(t *testing.T)) {
	t.Helper()
	t.Run(name, func(t *testing.T) {
		t.Helper()
		t.Log("case:", name)
		fn(t)
	})
}

@dnephin dnephin mentioned this issue Mar 15, 2021
Merged
qingyunha added a commit to qingyunha/go that referenced this issue Mar 17, 2021
@qingyunha
testing: fix Helper line number change
b00dcf0 
parent's helperNames has not been set when frameSkip called, moving
helperNames initilazing to frameSkip.

Fixes golang#44887

Change-Id: I5107c5951033e5e47d1ac441eac3ba5344a7bdc0
@qingyunha qingyunha mentioned this issue Mar 17, 2021
Closed
@gopherbot
Copy link

Change https://golang.org/cl/302469 mentions this issue: testing: fix Helper line number change

qingyunha added a commit to qingyunha/go that referenced this issue Mar 17, 2021
@qingyunha
testing: fix Helper line number change
b49843a 
parent's helperNames has not been set when frameSkip called, moving
helperNames initilazing to frameSkip.

Fixes golang#44887
qingyunha added a commit to qingyunha/go that referenced this issue Mar 17, 2021
@qingyunha
testing: fix Helper line number change
8f3c2bc 
parent's helperNames has not been set when frameSkip called, moving
helperNames initilazing to frameSkip.

Fixes golang#44887
qingyunha added a commit to qingyunha/go that referenced this issue Mar 17, 2021
@qingyunha
testing: update helperNames just before checking it
2ed2b81 
parent's helperNames has not been set when frameSkip called, moving
helperNames initilazing to frameSkip.

Fixes golang#44887
qingyunha added a commit to qingyunha/go that referenced this issue Mar 18, 2021
@qingyunha
testing: update helperNames just before checking it
44b90b2 
parent's helperNames has not been set when frameSkip called, moving
helperNames initilazing to frameSkip.

Fixes golang#44887
@gopherbot
Copy link

Change https://golang.org/cl/303189 mentions this issue: [release-branch.go1.16] testing: update helperNames just before checking it

gopherbot pushed a commit that referenced this issue Mar 24, 2021
@qingyunha @ianlancetaylor
[release-branch.go1.16] testing: update helperNames just before check…
902d16e 
…ing it

parent's helperNames has not been set when frameSkip called, moving
helperNames initilazing to frameSkip.

For #44887
Fixes #44888

Change-Id: I5107c5951033e5e47d1ac441eac3ba5344a7bdc0
GitHub-Last-Rev: 44b90b2
GitHub-Pull-Request: #45071
Reviewed-on: https://go-review.googlesource.com/c/go/+/302469
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
(cherry picked from commit 6704843)
Reviewed-on: https://go-review.googlesource.com/c/go/+/303189
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
@hgsgtk hgsgtk mentioned this issue Apr 5, 2021
Closed
@golang golang locked and limited conversation to collaborators Mar 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Milestone
Go1.17
Development

Successfully merging a pull request may close this issue.

testing: update helperNames just before checking it qingyunha/go
3 participants
@dnephin @ianlancetaylor @gopherbot