Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(727)

Side by Side Diff: src/pkg/net/smtp/smtp_test.go

Issue 6944057: code review 6944057: net/smtp: remove data race from TestSendMail. (Closed)
Patch Set: diff -r 5083a8d8cc89 https://code.google.com/p/go Created 11 years, 3 months ago
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments. Please Sign in to add in-line comments.
Jump to:
View unified diff | Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright 2010 The Go Authors. All rights reserved. 1 // Copyright 2010 The Go Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style 2 // Use of this source code is governed by a BSD-style
3 // license that can be found in the LICENSE file. 3 // license that can be found in the LICENSE file.
4 4
5 package smtp 5 package smtp
6 6
7 import ( 7 import (
8 "bufio" 8 "bufio"
9 "bytes" 9 "bytes"
10 "io" 10 "io"
11 "net" 11 "net"
12 "net/textproto" 12 "net/textproto"
13 "strings" 13 "strings"
14 "sync"
14 "testing" 15 "testing"
15 "time" 16 "time"
16 ) 17 )
17 18
18 type authTest struct { 19 type authTest struct {
19 auth Auth 20 auth Auth
20 challenges []string 21 challenges []string
21 name string 22 name string
22 responses []string 23 responses []string
23 } 24 }
(...skipping 354 matching lines...) Expand 10 before | Expand all | Expand 10 after
378 server := strings.Join(strings.Split(sendMailServer, "\n"), "\r\n") 379 server := strings.Join(strings.Split(sendMailServer, "\n"), "\r\n")
379 client := strings.Join(strings.Split(sendMailClient, "\n"), "\r\n") 380 client := strings.Join(strings.Split(sendMailClient, "\n"), "\r\n")
380 var cmdbuf bytes.Buffer 381 var cmdbuf bytes.Buffer
381 bcmdbuf := bufio.NewWriter(&cmdbuf) 382 bcmdbuf := bufio.NewWriter(&cmdbuf)
382 l, err := net.Listen("tcp", "127.0.0.1:0") 383 l, err := net.Listen("tcp", "127.0.0.1:0")
383 if err != nil { 384 if err != nil {
384 t.Fatalf("Unable to to create listener: %v", err) 385 t.Fatalf("Unable to to create listener: %v", err)
385 } 386 }
386 defer l.Close() 387 defer l.Close()
387 388
388 » go func(l net.Listener, data []string, w *bufio.Writer) { 389 » var mu sync.Mutex
bradfitz 2012/12/17 04:48:20 what is this guarding? it's not obvious. add a c
dfc 2012/12/17 04:49:29 you can also use a channel to synchronise here va
rick 2012/12/17 04:57:14 Done
rick 2012/12/17 04:57:14 Done.
390 » go func(l net.Listener, data []string, w *bufio.Writer, mu *sync.Mutex) {
dfc 2012/12/17 04:49:29 you don't need to pass w and mu into the function,
rick 2012/12/17 04:57:14 Done. Also removed l.
389 i := 0 391 i := 0
390 conn, err := l.Accept() 392 conn, err := l.Accept()
391 if err != nil { 393 if err != nil {
392 t.Log("Accept error: %v", err) 394 t.Log("Accept error: %v", err)
393 return 395 return
394 } 396 }
395 defer conn.Close() 397 defer conn.Close()
396 398
399 mu.Lock()
400 defer mu.Unlock()
397 tc := textproto.NewConn(conn) 401 tc := textproto.NewConn(conn)
398 for i < len(data) && data[i] != "" { 402 for i < len(data) && data[i] != "" {
399 tc.PrintfLine(data[i]) 403 tc.PrintfLine(data[i])
400 for len(data[i]) >= 4 && data[i][3] == '-' { 404 for len(data[i]) >= 4 && data[i][3] == '-' {
401 i++ 405 i++
402 tc.PrintfLine(data[i]) 406 tc.PrintfLine(data[i])
403 } 407 }
408 if data[i] == "221 Goodbye" {
409 return
410 }
404 read := false 411 read := false
405 for !read || data[i] == "354 Go ahead" { 412 for !read || data[i] == "354 Go ahead" {
406 msg, err := tc.ReadLine() 413 msg, err := tc.ReadLine()
407 w.Write([]byte(msg + "\r\n")) 414 w.Write([]byte(msg + "\r\n"))
408 read = true 415 read = true
409 if err != nil { 416 if err != nil {
410 t.Log("Read error: %v", err) 417 t.Log("Read error: %v", err)
411 return 418 return
412 } 419 }
413 if data[i] == "354 Go ahead" && msg == "." { 420 if data[i] == "354 Go ahead" && msg == "." {
414 break 421 break
415 } 422 }
416 } 423 }
417 i++ 424 i++
418 } 425 }
419 » }(l, strings.Split(server, "\r\n"), bcmdbuf) 426 » }(l, strings.Split(server, "\r\n"), bcmdbuf, &mu)
bradfitz 2012/12/17 04:48:20 no need to pass this to the goroutine. the gorout
rick 2012/12/17 04:57:14 Done
420 427
421 err = SendMail(l.Addr().String(), nil, "test@example.com", []string{"oth er@example.com"}, []byte(strings.Replace(`From: test@example.com 428 err = SendMail(l.Addr().String(), nil, "test@example.com", []string{"oth er@example.com"}, []byte(strings.Replace(`From: test@example.com
422 To: other@example.com 429 To: other@example.com
423 Subject: SendMail test 430 Subject: SendMail test
424 431
425 SendMail is working for me. 432 SendMail is working for me.
426 `, "\n", "\r\n", -1))) 433 `, "\n", "\r\n", -1)))
427 434
428 if err != nil { 435 if err != nil {
429 t.Errorf("%v", err) 436 t.Errorf("%v", err)
430 } 437 }
431 438
439 mu.Lock()
440 defer mu.Unlock()
432 bcmdbuf.Flush() 441 bcmdbuf.Flush()
433 actualcmds := cmdbuf.String() 442 actualcmds := cmdbuf.String()
434 if client != actualcmds { 443 if client != actualcmds {
435 t.Errorf("Got:\n%s\nExpected:\n%s", actualcmds, client) 444 t.Errorf("Got:\n%s\nExpected:\n%s", actualcmds, client)
436 } 445 }
437 } 446 }
438 447
439 var sendMailServer = `220 hello world 448 var sendMailServer = `220 hello world
440 502 EH? 449 502 EH?
441 250 mx.google.com at your service 450 250 mx.google.com at your service
(...skipping 10 matching lines...) Expand all
452 RCPT TO:<other@example.com> 461 RCPT TO:<other@example.com>
453 DATA 462 DATA
454 From: test@example.com 463 From: test@example.com
455 To: other@example.com 464 To: other@example.com
456 Subject: SendMail test 465 Subject: SendMail test
457 466
458 SendMail is working for me. 467 SendMail is working for me.
459 . 468 .
460 QUIT 469 QUIT
461 ` 470 `
OLDNEW
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b