[tools] gopls/internal/test/integration/completion: fix flaky TestUnimportedCompletion_VSCodeIssue3365
5 views
Skip to first unread message
Francisco Ferraz (Gerrit)
Feb 23, 2026, 3:08:49 PM (2 days ago) Feb 23
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Francisco Ferraz has uploaded the change for review![]( "Open in Gerrit")
Commit message
gopls/internal/test/integration/completion: fix flaky TestUnimportedCompletion_VSCodeIssue3365
The test used nested t.Run() subtests inside a runner.Run callback.
Env methods called e.TB.Fatal() on the parent subtest's t from within
a child subtest goroutine, causing "FailNow on a parent test" panics
and 20-minute timeouts.
Replace the inner t.Run() subtests with a plain loop.
Fixes golang/go#77756
Change-Id: I50cd5207dbe022a19fdd96e48d4a86e92710792c
Change diff
diff --git a/go/analysis/passes/modernize/stringscut.go b/go/analysis/passes/modernize/stringscut.go
index f963b54..1e670d9 100644
--- a/go/analysis/passes/modernize/stringscut.go
+++ b/go/analysis/passes/modernize/stringscut.go
@@ -183,7 +183,7 @@
// len(substr)]), then we can replace the call to Index()
// with a call to Cut() and use the returned ok, before,
// and after variables accordingly.
- negative, nonnegative, beforeSlice, afterSlice := checkIdxUses(pass.TypesInfo, index.Uses(iObj), s, substr)
+ negative, nonnegative, beforeSlice, afterSlice := checkIdxUses(pass.TypesInfo, index.Uses(iObj), s, substr, iObj)
// Either there are no uses of before, after, or ok, or some use
// of i does not match our criteria - don't suggest a fix.
@@ -374,7 +374,15 @@
// 2. nonnegative - a condition equivalent to i >= 0
// 3. beforeSlice - a slice of `s` that matches either s[:i], s[0:i]
// 4. afterSlice - a slice of `s` that matches one of: s[i+len(substr):], s[len(substr) + i:], s[i + const], s[k + i] (where k = len(substr))
-func checkIdxUses(info *types.Info, uses iter.Seq[inspector.Cursor], s, substr ast.Expr) (negative, nonnegative, beforeSlice, afterSlice []ast.Expr) {
+//
+// Additionally, all beforeSlice and afterSlice uses must be dominated by a
+// nonnegative guard on i (i.e., inside the body of an if whose condition
+// checks i >= 0, or in the else of a negative check, or after an
+// early-return negative check). This ensures that the rewrite from
+// s[i+len(sep):] to "after" preserves semantics, since when i == -1,
+// s[i+len(sep):] may yield a valid substring (e.g. s[0:] for single-byte
+// separators), but "after" would be "".
+func checkIdxUses(info *types.Info, uses iter.Seq[inspector.Cursor], s, substr ast.Expr, iObj types.Object) (negative, nonnegative, beforeSlice, afterSlice []ast.Expr) {
use := func(cur inspector.Cursor) bool {
ek := cur.ParentEdgeKind()
n := cur.Parent().Node()
@@ -398,9 +406,15 @@
sameObject(info, s, slice.X) &&
slice.Max == nil {
if isBeforeSlice(info, ek, slice) {
+ if !isSliceGuarded(cur, info, iObj) {
+ return false
+ }
beforeSlice = append(beforeSlice, slice)
return true
} else if isAfterSlice(info, ek, slice, substr) {
+ if !isSliceGuarded(cur, info, iObj) {
+ return false
+ }
afterSlice = append(afterSlice, slice)
return true
}
@@ -411,9 +425,15 @@
// have a max index.
if sameObject(info, s, slice.X) && slice.Max == nil {
if isBeforeSlice(info, ek, slice) {
+ if !isSliceGuarded(cur, info, iObj) {
+ return false
+ }
beforeSlice = append(beforeSlice, slice)
return true
} else if isAfterSlice(info, ek, slice, substr) {
+ if !isSliceGuarded(cur, info, iObj) {
+ return false
+ }
afterSlice = append(afterSlice, slice)
return true
}
@@ -578,6 +598,92 @@
return false
}
+// isSliceGuarded reports whether a use of the index variable i (at the given cursor)
+// inside a slice expression is dominated by a nonnegative guard.
+// A use is considered guarded if any of the following are true:
+// - It is inside the Body of an IfStmt whose condition is a nonnegative check on i.
+// - It is inside the Else of an IfStmt whose condition is a negative check on i.
+// - It is preceded (in the same block) by an IfStmt whose condition is a
+// negative check on i with a terminating body (e.g., early return).
+//
+// Conversely, a use is immediately rejected if:
+// - It is inside the Body of an IfStmt whose condition is a negative check on i.
+// - It is inside the Else of an IfStmt whose condition is a nonnegative check on i.
+func isSliceGuarded(cur inspector.Cursor, info *types.Info, iObj types.Object) bool {
+ for anc := range cur.Enclosing() {
+ switch anc.ParentEdgeKind() {
+ case edge.IfStmt_Body:
+ ifStmt := anc.Parent().Node().(*ast.IfStmt)
+ check := condChecksIdx(info, ifStmt.Cond, iObj)
+ if check > 0 {
+ return true // inside nonnegative-guarded body
+ }
+ if check < 0 {
+ return false // inside negative-guarded body (i < 0 here)
+ }
+ case edge.IfStmt_Else:
+ ifStmt := anc.Parent().Node().(*ast.IfStmt)
+ check := condChecksIdx(info, ifStmt.Cond, iObj)
+ if check < 0 {
+ return true // inside else of negative-guarded if (i >= 0 here)
+ }
+ if check > 0 {
+ return false // inside else of nonnegative-guarded if (i < 0 here)
+ }
+ case edge.BlockStmt_List:
+ // Check preceding siblings for early-return negative checks.
+ for sib, ok := anc.PrevSibling(); ok; sib, ok = sib.PrevSibling() {
+ ifStmt, ok := sib.Node().(*ast.IfStmt)
+ if ok && condChecksIdx(info, ifStmt.Cond, iObj) < 0 && bodyTerminates(ifStmt.Body) {
+ return true // preceded by early-return negative check
+ }
+ }
+ case edge.FuncDecl_Body, edge.FuncLit_Body:
+ return false // stop at function boundary
+ }
+ }
+ return false
+}
+
+// condChecksIdx reports whether cond is a BinaryExpr that checks
+// the index variable iObj for negativity or non-negativity.
+// Returns -1 for negative (e.g. i < 0), +1 for nonnegative (e.g. i >= 0), 0 otherwise.
+func condChecksIdx(info *types.Info, cond ast.Expr, iObj types.Object) int {
+ binExpr, ok := cond.(*ast.BinaryExpr)
+ if !ok {
+ return 0
+ }
+ var refersToIdx bool
+ if ident, ok := binExpr.X.(*ast.Ident); ok {
+ refersToIdx = info.ObjectOf(ident) == iObj
+ }
+ if !refersToIdx {
+ if ident, ok := binExpr.Y.(*ast.Ident); ok {
+ refersToIdx = info.ObjectOf(ident) == iObj
+ }
+ }
+ if !refersToIdx {
+ return 0
+ }
+ return checkIdxComparison(info, binExpr)
+}
+
+// bodyTerminates reports whether the given block statement unconditionally
+// terminates execution (via return, break, continue, or goto).
+func bodyTerminates(block *ast.BlockStmt) bool {
+ if len(block.List) == 0 {
+ return false
+ }
+ last := block.List[len(block.List)-1]
+ switch last.(type) {
+ case *ast.ReturnStmt:
+ return true
+ case *ast.BranchStmt:
+ return true // break, continue, goto
+ }
+ return false
+}
+
// sameObject reports whether we know that the expressions resolve to the same object.
func sameObject(info *types.Info, expr1, expr2 ast.Expr) bool {
if ident1, ok := expr1.(*ast.Ident); ok {
diff --git a/go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go b/go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go
index 5990771..d91591a 100644
--- a/go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go
+++ b/go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go
@@ -7,7 +7,7 @@
func basic(s string) bool {
s = "reassigned"
- i := strings.Index(s, "=") // want "strings.Index can be simplified using strings.Cut"
+ i := strings.Index(s, "=") // don't modernize: s[:i] is not guarded by i >= 0
print(s[:i])
return i >= 0
}
@@ -49,7 +49,7 @@
}
func basic_substr_arg(s string, substr string) bool {
- i := strings.Index(s, substr) // want "strings.Index can be simplified using strings.Cut"
+ i := strings.Index(s, substr) // don't modernize: s[i+len(substr):] is not guarded by i >= 0
print(s[i+len(substr):])
return i >= 0
}
@@ -61,26 +61,26 @@
}
func basic_strings_byte(s string) bool {
- i := strings.IndexByte(s, '+') // want "strings.IndexByte can be simplified using strings.Cut"
+ i := strings.IndexByte(s, '+') // don't modernize: s[:i] is not guarded by i >= 0
print(s[:i])
return i >= 0
}
func basic_strings_byte_int(s string) bool {
- i := strings.IndexByte(s, 55) // want "strings.IndexByte can be simplified using strings.Cut"
+ i := strings.IndexByte(s, 55) // don't modernize: s[:i] is not guarded by i >= 0
print(s[:i])
return i >= 0
}
func basic_strings_byte_var(s string) bool {
b := byte('b')
- i := strings.IndexByte(s, b) // want "strings.IndexByte can be simplified using strings.Cut"
+ i := strings.IndexByte(s, b) // don't modernize: s[:i] is not guarded by i >= 0
print(s[:i])
return i >= 0
}
func basic_bytes(b []byte) []byte {
- i := bytes.Index(b, []byte("str")) // want "bytes.Index can be simplified using bytes.Cut"
+ i := bytes.Index(b, []byte("str")) // don't modernize: b[i+3:] in else is not guarded
if i >= 0 {
return b[:i]
} else {
@@ -89,7 +89,7 @@
}
func basic_index_bytes(b []byte) string {
- i := bytes.IndexByte(b, 's') // want "bytes.IndexByte can be simplified using bytes.Cut"
+ i := bytes.IndexByte(b, 's') // don't modernize: b[i+1:] in else is not guarded
if i >= 0 {
return string(b[:i])
} else {
@@ -98,13 +98,13 @@
}
func const_substr_len(s string) bool {
- i := strings.Index(s, "=") // want "strings.Index can be simplified using strings.Cut"
+ i := strings.Index(s, "=") // don't modernize: s[i+len("="):] is not guarded
r := s[i+len("="):]
return len(r) > 0
}
func const_for_len(s string) bool {
- i := strings.Index(s, "=") // want "strings.Index can be simplified using strings.Cut"
+ i := strings.Index(s, "=") // don't modernize: s[i+1:] is not guarded
r := s[i+1:]
return len(r) > 0
}
@@ -154,7 +154,7 @@
func index_and_before_after(s string) string {
substr := "="
- i := strings.Index(s, substr) // want "strings.Index can be simplified using strings.Cut"
+ i := strings.Index(s, substr) // don't modernize: s[i+len(substr):] and s[len(substr)+i:] not nonneg-guarded
if i == -1 {
print("test")
}
@@ -174,7 +174,7 @@
}
func idx_var_init(s string) (string, string) {
- var idx = strings.Index(s, "=") // want "strings.Index can be simplified using strings.Cut"
+ var idx = strings.Index(s, "=") // don't modernize: s[0:idx] is not guarded
return s[0:idx], s
}
@@ -219,7 +219,7 @@
func s_in_func_call() string {
s := "string"
substr := "substr"
- idx := strings.Index(s, substr) // want "strings.Index can be simplified using strings.Cut"
+ idx := strings.Index(s, substr) // don't modernize: s[:idx] is not guarded
function(s)
return s[:idx]
}
@@ -306,3 +306,51 @@
func reference_int(i *int) {}
func blank() {}
+
+// Regression test for unguarded slice uses (https://go.dev/issue/77737).
+// The s[colon+1:] usage outside the "if" relies on -1+1=0 to return the full
+// string when the separator is absent. Rewriting to "after" would return "".
+func unguarded_after_slice(s string) (int, string) {
+ colon := strings.Index(s, ":")
+ if colon != -1 {
+ print(s[:colon])
+ }
+ return colon, s[colon+1:] // don't modernize: s[colon+1:] not guarded
+}
+
+// Same as above but with the guard using i < 0.
+func unguarded_after_slice_negcheck(s string) string {
+ i := strings.Index(s, ":")
+ if i < 0 {
+ print("not found")
+ }
+ return s[i+1:] // don't modernize: s[i+1:] not guarded
+}
+
+// Safe: both slice uses are inside the nonneg guard.
+func guarded_both_slices(s string) (string, string) {
+ i := strings.Index(s, ":") // want "strings.Index can be simplified using strings.Cut"
+ if i >= 0 {
+ return s[:i], s[i+1:]
+ }
+ return "", s
+}
+
+// Safe: slice uses after early-return negative check.
+func guarded_early_return(s string) (string, string) {
+ i := strings.Index(s, ":") // want "strings.Index can be simplified using strings.Cut"
+ if i < 0 {
+ return "", s
+ }
+ return s[:i], s[i+1:]
+}
+
+// Safe: slice uses in else of negative check.
+func guarded_neg_else(s string) (string, string) {
+ i := strings.Index(s, ":") // want "strings.Index can be simplified using strings.Cut"
+ if i == -1 {
+ return "", s
+ } else {
+ return s[:i], s[i+1:]
+ }
+}
diff --git a/go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go.golden b/go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go.golden
index e274382..eb26502 100644
--- a/go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go.golden
+++ b/go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go.golden
@@ -7,9 +7,9 @@
func basic(s string) bool {
s = "reassigned"
- before, _, ok := strings.Cut(s, "=") // want "strings.Index can be simplified using strings.Cut"
- print(before)
- return ok
+ i := strings.Index(s, "=") // don't modernize: s[:i] is not guarded by i >= 0
+ print(s[:i])
+ return i >= 0
}
func basic_contains(s string) bool {
@@ -49,9 +49,9 @@
}
func basic_substr_arg(s string, substr string) bool {
- _, after, ok := strings.Cut(s, substr) // want "strings.Index can be simplified using strings.Cut"
- print(after)
- return ok
+ i := strings.Index(s, substr) // don't modernize: s[i+len(substr):] is not guarded by i >= 0
+ print(s[i+len(substr):])
+ return i >= 0
}
func wrong_len_arg(s string, substr string) bool {
@@ -61,51 +61,51 @@
}
func basic_strings_byte(s string) bool {
- before, _, ok := strings.Cut(s, "+") // want "strings.IndexByte can be simplified using strings.Cut"
- print(before)
- return ok
+ i := strings.IndexByte(s, '+') // don't modernize: s[:i] is not guarded by i >= 0
+ print(s[:i])
+ return i >= 0
}
func basic_strings_byte_int(s string) bool {
- before, _, ok := strings.Cut(s, "7") // want "strings.IndexByte can be simplified using strings.Cut"
- print(before)
- return ok
+ i := strings.IndexByte(s, 55) // don't modernize: s[:i] is not guarded by i >= 0
+ print(s[:i])
+ return i >= 0
}
func basic_strings_byte_var(s string) bool {
b := byte('b')
- before, _, ok := strings.Cut(s, string(b)) // want "strings.IndexByte can be simplified using strings.Cut"
- print(before)
- return ok
+ i := strings.IndexByte(s, b) // don't modernize: s[:i] is not guarded by i >= 0
+ print(s[:i])
+ return i >= 0
}
func basic_bytes(b []byte) []byte {
- before, after, ok := bytes.Cut(b, []byte("str")) // want "bytes.Index can be simplified using bytes.Cut"
- if ok {
- return before
+ i := bytes.Index(b, []byte("str")) // don't modernize: b[i+3:] in else is not guarded
+ if i >= 0 {
+ return b[:i]
} else {
- return after
+ return b[i+3:]
}
}
func basic_index_bytes(b []byte) string {
- before, after, ok := bytes.Cut(b, []byte{'s'}) // want "bytes.IndexByte can be simplified using bytes.Cut"
- if ok {
- return string(before)
+ i := bytes.IndexByte(b, 's') // don't modernize: b[i+1:] in else is not guarded
+ if i >= 0 {
+ return string(b[:i])
} else {
- return string(after)
+ return string(b[i+1:])
}
}
func const_substr_len(s string) bool {
- _, after, _ := strings.Cut(s, "=") // want "strings.Index can be simplified using strings.Cut"
- r := after
+ i := strings.Index(s, "=") // don't modernize: s[i+len("="):] is not guarded
+ r := s[i+len("="):]
return len(r) > 0
}
func const_for_len(s string) bool {
- _, after, _ := strings.Cut(s, "=") // want "strings.Index can be simplified using strings.Cut"
- r := after
+ i := strings.Index(s, "=") // don't modernize: s[i+1:] is not guarded
+ r := s[i+1:]
return len(r) > 0
}
@@ -154,28 +154,28 @@
func index_and_before_after(s string) string {
substr := "="
- before, after, ok := strings.Cut(s, substr) // want "strings.Index can be simplified using strings.Cut"
- if !ok {
+ i := strings.Index(s, substr) // don't modernize: s[i+len(substr):] and s[len(substr)+i:] not nonneg-guarded
+ if i == -1 {
print("test")
}
- if !ok {
+ if i < 0 {
return ""
} else {
- if ok {
- return before
+ if i >= 0 {
+ return s[:i]
} else {
- return after
+ return s[i+len(substr):]
}
}
- if !ok {
- return after
+ if -1 == i {
+ return s[len(substr)+i:]
}
return "final"
}
func idx_var_init(s string) (string, string) {
- var before, _, _ = strings.Cut(s, "=") // want "strings.Index can be simplified using strings.Cut"
- return before, s
+ var idx = strings.Index(s, "=") // don't modernize: s[0:idx] is not guarded
+ return s[0:idx], s
}
func idx_reassigned(s string) string {
@@ -219,9 +219,9 @@
func s_in_func_call() string {
s := "string"
substr := "substr"
- before, _, _ := strings.Cut(s, substr) // want "strings.Index can be simplified using strings.Cut"
+ idx := strings.Index(s, substr) // don't modernize: s[:idx] is not guarded
function(s)
- return before
+ return s[:idx]
}
func s_pointer() string {
@@ -305,4 +305,52 @@
func reference_int(i *int) {}
-func blank() {}
\ No newline at end of file
+func blank() {}
+
+// Regression test for unguarded slice uses (https://go.dev/issue/77737).
+// The s[colon+1:] usage outside the "if" relies on -1+1=0 to return the full
+// string when the separator is absent. Rewriting to "after" would return "".
+func unguarded_after_slice(s string) (int, string) {
+ colon := strings.Index(s, ":")
+ if colon != -1 {
+ print(s[:colon])
+ }
+ return colon, s[colon+1:] // don't modernize: s[colon+1:] not guarded
+}
+
+// Same as above but with the guard using i < 0.
+func unguarded_after_slice_negcheck(s string) string {
+ i := strings.Index(s, ":")
+ if i < 0 {
+ print("not found")
+ }
+ return s[i+1:] // don't modernize: s[i+1:] not guarded
+}
+
+// Safe: both slice uses are inside the nonneg guard.
+func guarded_both_slices(s string) (string, string) {
+ before, after, ok := strings.Cut(s, ":") // want "strings.Index can be simplified using strings.Cut"
+ if ok {
+ return before, after
+ }
+ return "", s
+}
+
+// Safe: slice uses after early-return negative check.
+func guarded_early_return(s string) (string, string) {
+ before, after, ok := strings.Cut(s, ":") // want "strings.Index can be simplified using strings.Cut"
+ if !ok {
+ return "", s
+ }
+ return before, after
+}
+
+// Safe: slice uses in else of negative check.
+func guarded_neg_else(s string) (string, string) {
+ before, after, ok := strings.Cut(s, ":") // want "strings.Index can be simplified using strings.Cut"
+ if !ok {
+ return "", s
+ } else {
+ return before, after
+ }
+}
diff --git a/gopls/internal/test/integration/completion/completion_test.go b/gopls/internal/test/integration/completion/completion_test.go
index a50b5d9..d97b914 100644
--- a/gopls/internal/test/integration/completion/completion_test.go
+++ b/gopls/internal/test/integration/completion/completion_test.go
@@ -706,20 +706,17 @@
}
for _, tc := range testcases {
- t.Run(fmt.Sprintf("%v/%v", tc.mode, tc.accept), func(t *testing.T) {
-
- env.SetSuggestionInsertReplaceMode(tc.mode == "replace")
- env.SetBufferContent("main.go", orig)
- loc := env.RegexpSearch("main.go", `()Lower\)`)
- completions := env.Completion(loc)
- item := find(t, completions, tc.accept)
- env.AcceptCompletion(loc, item)
- env.Await(env.DoneWithChange())
- got := env.BufferText("main.go")
- if !strings.Contains(got, tc.want) {
- t.Errorf("unexpected state after completion:\n%v\nwanted %v", got, tc.want)
- }
- })
+ env.SetSuggestionInsertReplaceMode(tc.mode == "replace")
+ env.SetBufferContent("main.go", orig)
+ loc := env.RegexpSearch("main.go", `()Lower\)`)
+ completions := env.Completion(loc)
+ item := find(t, completions, tc.accept)
+ env.AcceptCompletion(loc, item)
+ env.Await(env.DoneWithChange())
+ got := env.BufferText("main.go")
+ if !strings.Contains(got, tc.want) {
+ t.Errorf("%v/%v: unexpected state after completion:\n%v\nwanted %v", tc.mode, tc.accept, got, tc.want)
+ }
}
})
})
Change information
Files:
- M go/analysis/passes/modernize/stringscut.go
- M go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go
- M go/analysis/passes/modernize/testdata/src/stringscut/stringscut.go.golden
- M gopls/internal/test/integration/completion/completion_test.go
Change size: L
Delta: 4 files changed, 267 insertions(+), 68 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Francisco Ferraz (Gerrit)
Feb 23, 2026, 3:14:11 PM (2 days ago) Feb 23
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Francisco Ferraz uploaded new patchset![]( "Open in Gerrit")
Francisco Ferraz uploaded patch set #2 to this change.
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Feb 23, 2026, 8:18:43 PM (2 days ago) Feb 23
to Francisco Ferraz, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Francisco Ferraz
Alan Donovan added 2 comments![]( "Open in Gerrit")
Commit Message
Line 9, Patchset 2 (Latest):
The test used nested t.Run() subtests inside a runner.Run callback.Alan Donovan . resolved
Every time we capture a testing.T inside some other structure we seem to be setting ourselves up for trouble. Thanks for investigating!
File gopls/internal/test/integration/completion/completion_test.go
Line 709, Patchset 2 (Latest):
env.SetSuggestionInsertReplaceMode(tc.mode == "replace")Alan Donovan . unresolved
Could you also t.Log the mode and accept values here, just in case any of the steps before t.Errorf should fail? Thanks.
Related details
Attention is currently required from:
- Francisco Ferraz
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Francisco Ferraz (Gerrit)
Feb 24, 2026, 4:58:57 AM (yesterday) Feb 24
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Francisco Ferraz
Francisco Ferraz uploaded new patchset![]( "Open in Gerrit")
Francisco Ferraz uploaded patch set #3 to this change.
Related details
Attention is currently required from:
- Francisco Ferraz
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Francisco Ferraz (Gerrit)
Feb 24, 2026, 5:01:26 AM (24 hours ago) Feb 24
to goph...@pubsubhelper.golang.org, Alan Donovan, golang-co...@googlegroups.com
Attention needed from Alan Donovan
Francisco Ferraz added 1 comment![]( "Open in Gerrit")
File gopls/internal/test/integration/completion/completion_test.go
Line 709, Patchset 2:
env.SetSuggestionInsertReplaceMode(tc.mode == "replace")Alan Donovan . resolved
Could you also t.Log the mode and accept values here, just in case any of the steps before t.Errorf should fail? Thanks.
Attention is currently required from:
- Alan Donovan
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Feb 24, 2026, 12:17:48 PM (17 hours ago) Feb 24
to Francisco Ferraz, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Francisco Ferraz
Alan Donovan voted and added 1 comment![]( "Open in Gerrit")
Votes added by Alan Donovan
| Auto-Submit | +1 |
| Code-Review | +2 |
| Commit-Queue | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Francisco Ferraz
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
David Chase (Gerrit)
Feb 24, 2026, 3:48:03 PM (13 hours ago) Feb 24
to Francisco Ferraz, goph...@pubsubhelper.golang.org, Go LUCI, Madeline Kalil, Alan Donovan, golang-co...@googlegroups.com
Attention needed from Francisco Ferraz
David Chase voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Francisco Ferraz
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gopher Robot (Gerrit)
Feb 24, 2026, 3:48:44 PM (13 hours ago) Feb 24
to Francisco Ferraz, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, David Chase, Go LUCI, Madeline Kalil, Alan Donovan, golang-co...@googlegroups.com
Gopher Robot submitted the change![]( "Open in Gerrit")
Change information
Commit message:
gopls/internal/test/integration/completion: fix flaky TestUnimportedCompletion_VSCodeIssue3365
The test used nested t.Run() subtests inside a runner.Run callback.
Env methods called e.TB.Fatal() on the parent subtest's t from within
a child subtest goroutine, causing "FailNow on a parent test" panics
and 20-minute timeouts.
Replace the inner t.Run() subtests with a plain loop.
Fixes golang/go#77756
Change-Id: I50cd5207dbe022a19fdd96e48d4a86e92710792c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/747461
Auto-Submit: Alan Donovan <adon...@google.com>
Reviewed-by: David Chase <drc...@google.com>
LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adon...@google.com>
Files:
- M gopls/internal/test/integration/completion/completion_test.go
Change size: XS
Delta: 1 file changed, 1 insertion(+), 1 deletion(-)
Branch: refs/heads/master
Submit Requirements:
Code-Review: +1 by David Chase, +2 by Alan Donovan
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Madeline Kalil (Gerrit)
Feb 24, 2026, 4:01:13 PM (13 hours ago) Feb 24
to Gopher Robot, Francisco Ferraz, goph...@pubsubhelper.golang.org, David Chase, Go LUCI, Alan Donovan, golang-co...@googlegroups.com
Attention needed from Francisco Ferraz
Madeline Kalil added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 4 (Latest):
Madeline Kalil . unresolved
@francisc...@gmail.com Looks like some diffs got lost in the final patch set (t.Run() doesn't get removed), would you be able to send another CL?
Related details
Attention is currently required from:
- Francisco Ferraz
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Francisco Ferraz (Gerrit)
Feb 24, 2026, 4:19:34 PM (13 hours ago) Feb 24
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Francisco Ferraz has uploaded the change for review![]( "Open in Gerrit")
Commit message
gopls/internal/test/integration/completion: fix flaky TestUnimportedCompletion_VSCodeIssue3365
The test used nested t.Run() subtests inside a runner.Run callback.
Env methods called e.TB.Fatal() on the parent subtest's t from within
a child subtest goroutine, causing "FailNow on a parent test" panics
and 20-minute timeouts.
Replace the inner t.Run() subtests with a plain loop.
Fixes golang/go#77756
Change-Id: I37e1c77606981665bdb762316da3b02b5c6505fc
Change diff
diff --git a/gopls/internal/test/integration/completion/completion_test.go b/gopls/internal/test/integration/completion/completion_test.go
index a50b5d9..4ddc7cb 100644
--- a/gopls/internal/test/integration/completion/completion_test.go
+++ b/gopls/internal/test/integration/completion/completion_test.go
@@ -706,20 +706,17 @@
}
for _, tc := range testcases {
- t.Run(fmt.Sprintf("%v/%v", tc.mode, tc.accept), func(t *testing.T) {
-
- env.SetSuggestionInsertReplaceMode(tc.mode == "replace")
- env.SetBufferContent("main.go", orig)
- loc := env.RegexpSearch("main.go", `()Lower\)`)
- completions := env.Completion(loc)
- item := find(t, completions, tc.accept)
- env.AcceptCompletion(loc, item)
- env.Await(env.DoneWithChange())
- got := env.BufferText("main.go")
- if !strings.Contains(got, tc.want) {
- t.Errorf("unexpected state after completion:\n%v\nwanted %v", got, tc.want)
- }
- })
+ t.Logf("mode=%q accept=%q", tc.mode, tc.accept)
+ env.SetSuggestionInsertReplaceMode(tc.mode == "replace")
+ env.SetBufferContent("main.go", orig)
+ loc := env.RegexpSearch("main.go", `()Lower\)`)
+ completions := env.Completion(loc)
+ item := find(t, completions, tc.accept)
+ env.AcceptCompletion(loc, item)
+ got := env.BufferText("main.go")
+ if !strings.Contains(got, tc.want) {
+ t.Errorf("unexpected state after completion:\n%v\nwanted %v", got, tc.want)
+ }
}
})
})
Change information
Files:
- M gopls/internal/test/integration/completion/completion_test.go
Change size: S
Delta: 1 file changed, 11 insertions(+), 14 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Francisco Ferraz (Gerrit)
Feb 24, 2026, 4:28:36 PM (13 hours ago) Feb 24
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Francisco Ferraz uploaded new patchset![]( "Open in Gerrit")
Francisco Ferraz uploaded patch set #2 to this change.
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Francisco Ferraz (Gerrit)
Feb 24, 2026, 4:30:35 PM (13 hours ago) Feb 24
to Gopher Robot, goph...@pubsubhelper.golang.org, David Chase, Go LUCI, Madeline Kalil, Alan Donovan, golang-co...@googlegroups.com
Francisco Ferraz added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Madeline Kalil . resolved
@francisc...@gmail.com Looks like some diffs got lost in the final patch set (t.Run() doesn't get removed), would you be able to send another CL?
Francisco Ferraz
I cant seem to commit new changes since this was already merged.
I have pushed a new patch set with non-corrupted files.
https://go-review.googlesource.com/c/tools/+/748620
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Madeline Kalil (Gerrit)
Feb 24, 2026, 4:59:50 PM (12 hours ago) Feb 24
to Francisco Ferraz, goph...@pubsubhelper.golang.org, Alan Donovan, golang-co...@googlegroups.com
Attention needed from Alan Donovan and Francisco Ferraz
Madeline Kalil voted and added 1 comment![]( "Open in Gerrit")
Votes added by Madeline Kalil
| Code-Review | +2 |
| Commit-Queue | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Alan Donovan
- Francisco Ferraz
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Feb 24, 2026, 9:31:22 PM (7 hours ago) Feb 24
to Francisco Ferraz, goph...@pubsubhelper.golang.org, Go LUCI, Madeline Kalil, golang-co...@googlegroups.com
Attention needed from Francisco Ferraz
Alan Donovan voted and added 1 comment![]( "Open in Gerrit")
Votes added by Alan Donovan
| Auto-Submit | +1 |
| Code-Review | +2 |
| Commit-Queue | +1 |
1 comment
Patchset-level comments
Alan Donovan . resolved
Thanks!
Attention is currently required from:
- Francisco Ferraz
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Feb 24, 2026, 9:31:30 PM (7 hours ago) Feb 24
to Francisco Ferraz, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go LUCI, Madeline Kalil, golang-co...@googlegroups.com
Alan Donovan submitted the change![]( "Open in Gerrit")
Change information
Commit message:
gopls/internal/test/integration/completion: fix flaky TestUnimportedCompletion_VSCodeIssue3365
The test used nested t.Run() subtests inside a runner.Run callback.
Env methods called e.TB.Fatal() on the parent subtest's t from within
a child subtest goroutine, causing "FailNow on a parent test" panics
and 20-minute timeouts.
Replace the inner t.Run() subtests with a plain loop.
Fixes golang/go#77756
Change-Id: I37e1c77606981665bdb762316da3b02b5c6505fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/748620
Reviewed-by: Alan Donovan <adon...@google.com>
Commit-Queue: Alan Donovan <adon...@google.com>
LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Alan Donovan <adon...@google.com>
Reviewed-by: Madeline Kalil <mka...@google.com>
Files:
- M gopls/internal/test/integration/completion/completion_test.go
Change size: S
Delta: 1 file changed, 11 insertions(+), 14 deletions(-)
Branch: refs/heads/master
Submit Requirements:
Code-Review: +2 by Madeline Kalil, +2 by Alan Donovan
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages