Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit94eb9b8

Browse files
authored
fix: disable t.Parallel on TestPortForward (#10449)
I've said it before, I'll say it again: you can't create a timed context before calling `t.Parallel()` and then use it after.Fixes flakes likehttps://github.com/coder/coder/actions/runs/6716682414/job/18253279157I've chosen just to drop `t.Parallel()` entirely rather than create a second context after the parallel call, since the vast majority of the test time happens before where the parallel call was. It does all the tailnet setup before `t.Parallel()`.Leaving a call to `t.Parallel()` is a bug risk for future maintainers to come in and use the wrong context in the latter part of the test by accident.
1 parent6882e8e commit94eb9b8

File tree

3 files changed

+28
-10
lines changed

3 files changed

+28
-10
lines changed

‎cli/portforward.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ func (r *RootCmd) portForward() *clibase.Cmd {
9898
returnxerrors.Errorf("await agent: %w",err)
9999
}
100100

101-
varlogger slog.Logger
101+
logger:=slog.Make()
102102
ifr.verbose {
103103
logger=slog.Make(sloghuman.Sink(inv.Stdout)).Leveled(slog.LevelDebug)
104104
}
@@ -131,7 +131,7 @@ func (r *RootCmd) portForward() *clibase.Cmd {
131131
defercloseAllListeners()
132132

133133
fori,spec:=rangespecs {
134-
l,err:=listenAndPortForward(ctx,inv,conn,wg,spec)
134+
l,err:=listenAndPortForward(ctx,inv,conn,wg,spec,logger)
135135
iferr!=nil {
136136
returnerr
137137
}
@@ -185,7 +185,15 @@ func (r *RootCmd) portForward() *clibase.Cmd {
185185
returncmd
186186
}
187187

188-
funclistenAndPortForward(ctx context.Context,inv*clibase.Invocation,conn*codersdk.WorkspaceAgentConn,wg*sync.WaitGroup,specportForwardSpec) (net.Listener,error) {
188+
funclistenAndPortForward(
189+
ctx context.Context,
190+
inv*clibase.Invocation,
191+
conn*codersdk.WorkspaceAgentConn,
192+
wg*sync.WaitGroup,
193+
specportForwardSpec,
194+
logger slog.Logger,
195+
) (net.Listener,error) {
196+
logger=logger.With(slog.F("network",spec.listenNetwork),slog.F("address",spec.listenAddress))
189197
_,_=fmt.Fprintf(inv.Stderr,"Forwarding '%v://%v' locally to '%v://%v' in the workspace\n",spec.listenNetwork,spec.listenAddress,spec.dialNetwork,spec.dialAddress)
190198

191199
var (
@@ -218,6 +226,7 @@ func listenAndPortForward(ctx context.Context, inv *clibase.Invocation, conn *co
218226
iferr!=nil {
219227
returnnil,xerrors.Errorf("listen '%v://%v': %w",spec.listenNetwork,spec.listenAddress,err)
220228
}
229+
logger.Debug(ctx,"listening")
221230

222231
wg.Add(1)
223232
gofunc(specportForwardSpec) {
@@ -227,12 +236,14 @@ func listenAndPortForward(ctx context.Context, inv *clibase.Invocation, conn *co
227236
iferr!=nil {
228237
// Silently ignore net.ErrClosed errors.
229238
ifxerrors.Is(err,net.ErrClosed) {
239+
logger.Debug(ctx,"listener closed")
230240
return
231241
}
232242
_,_=fmt.Fprintf(inv.Stderr,"Error accepting connection from '%v://%v': %v\n",spec.listenNetwork,spec.listenAddress,err)
233243
_,_=fmt.Fprintln(inv.Stderr,"Killing listener")
234244
return
235245
}
246+
logger.Debug(ctx,"accepted connection",slog.F("remote_addr",netConn.RemoteAddr()))
236247

237248
gofunc(netConn net.Conn) {
238249
defernetConn.Close()
@@ -242,8 +253,10 @@ func listenAndPortForward(ctx context.Context, inv *clibase.Invocation, conn *co
242253
return
243254
}
244255
deferremoteConn.Close()
256+
logger.Debug(ctx,"dialed remote",slog.F("remote_addr",netConn.RemoteAddr()))
245257

246258
agentssh.Bicopy(ctx,netConn,remoteConn)
259+
logger.Debug(ctx,"connection closing",slog.F("remote_addr",netConn.RemoteAddr()))
247260
}(netConn)
248261
}
249262
}(spec)

‎cli/portforward_test.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,10 @@ func TestPortForward(t *testing.T) {
140140

141141
for_,c:=rangecases {
142142
c:=c
143-
//Delay parallel tests here because setupLocal reserves
143+
//No parallel tests here because setupLocal reserves
144144
// a free open port which is not guaranteed to be free
145145
// between the listener closing and port-forward ready.
146+
//nolint:tparallel,paralleltest
146147
t.Run(c.name+"_OnePort",func(t*testing.T) {
147148
p1:=setupTestListener(t,c.setupRemote(t))
148149

@@ -166,8 +167,6 @@ func TestPortForward(t *testing.T) {
166167
}()
167168
pty.ExpectMatchContext(ctx,"Ready!")
168169

169-
t.Parallel()// Port is reserved, enable parallel execution.
170-
171170
// Open two connections simultaneously and test them out of
172171
// sync.
173172
d:= net.Dialer{Timeout:testutil.WaitShort}
@@ -185,6 +184,10 @@ func TestPortForward(t *testing.T) {
185184
require.ErrorIs(t,err,context.Canceled)
186185
})
187186

187+
// No parallel tests here because setupLocal reserves
188+
// a free open port which is not guaranteed to be free
189+
// between the listener closing and port-forward ready.
190+
//nolint:tparallel,paralleltest
188191
t.Run(c.name+"_TwoPorts",func(t*testing.T) {
189192
var (
190193
p1=setupTestListener(t,c.setupRemote(t))
@@ -213,8 +216,6 @@ func TestPortForward(t *testing.T) {
213216
}()
214217
pty.ExpectMatchContext(ctx,"Ready!")
215218

216-
t.Parallel()// Port is reserved, enable parallel execution.
217-
218219
// Open a connection to both listener 1 and 2 simultaneously and
219220
// then test them out of order.
220221
d:= net.Dialer{Timeout:testutil.WaitShort}
@@ -234,6 +235,10 @@ func TestPortForward(t *testing.T) {
234235
}
235236

236237
// Test doing TCP and UDP at the same time.
238+
// No parallel tests here because setupLocal reserves
239+
// a free open port which is not guaranteed to be free
240+
// between the listener closing and port-forward ready.
241+
//nolint:tparallel,paralleltest
237242
t.Run("All",func(t*testing.T) {
238243
var (
239244
dials= []addr{}
@@ -266,8 +271,6 @@ func TestPortForward(t *testing.T) {
266271
}()
267272
pty.ExpectMatchContext(ctx,"Ready!")
268273

269-
t.Parallel()// Port is reserved, enable parallel execution.
270-
271274
// Open connections to all items in the "dial" array.
272275
var (
273276
d= net.Dialer{Timeout:testutil.WaitShort}

‎tailnet/conn.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -936,10 +936,12 @@ func (c *Conn) Listen(network, addr string) (net.Listener, error) {
936936
}
937937

938938
func (c*Conn)DialContextTCP(ctx context.Context,ipp netip.AddrPort) (*gonet.TCPConn,error) {
939+
c.logger.Debug(ctx,"dial tcp",slog.F("addr_port",ipp))
939940
returnc.netStack.DialContextTCP(ctx,ipp)
940941
}
941942

942943
func (c*Conn)DialContextUDP(ctx context.Context,ipp netip.AddrPort) (*gonet.UDPConn,error) {
944+
c.logger.Debug(ctx,"dial udp",slog.F("addr_port",ipp))
943945
returnc.netStack.DialContextUDP(ctx,ipp)
944946
}
945947

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp