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

Issue 45620043: code review 45620043: go.net/ipv6: add missing API tests (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 2 months ago by mikio
Modified:
11 years, 2 months ago
Reviewers:
dave
CC:
golang-codereviews, dave_cheney.net
Visibility:
Public.

Description

go.net/ipv6: add missing API tests Now the package has a code coverage of 76.1% of statements. Also fixes flaky concurrent test. Fixes issue 5696.

Patch Set 1 : diff -r 127da548775d https://code.google.com/p/go.net #

Total comments: 6

Patch Set 2 : diff -r e0d7bfced6e2 https://code.google.com/p/go.net #

Unified diffs Side-by-side diffs Delta from patch set Stats (+233 lines, -138 lines) Patch
R ipv6/control_test.go View 1 chunk +0 lines, -42 lines 0 comments Download
M ipv6/icmp_test.go View 1 chunk +18 lines, -0 lines 0 comments Download
M ipv6/multicast_test.go View 1 5 chunks +19 lines, -9 lines 0 comments Download
A ipv6/readwrite_test.go View 1 1 chunk +168 lines, -0 lines 0 comments Download
M ipv6/unicast_test.go View 7 chunks +28 lines, -87 lines 0 comments Download

Messages

Total messages: 4
mikio
Hello golang-codereviews@googlegroups.com (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go.net
11 years, 2 months ago (2013-12-26 08:17:12 UTC) #1
mikio
ping
11 years, 2 months ago (2013-12-31 08:22:17 UTC) #2
dave_cheney.net
LGTM with minor comments. https://codereview.appspot.com/45620043/diff/220001/ipv6/multicast_test.go File ipv6/multicast_test.go (right): https://codereview.appspot.com/45620043/diff/220001/ipv6/multicast_test.go#newcode39 ipv6/multicast_test.go:39: defer p.Close() Why did you ...
11 years, 2 months ago (2013-12-31 10:33:22 UTC) #3
mikio
11 years, 2 months ago (2013-12-31 14:25:03 UTC) #4
*** Submitted as
https://code.google.com/p/go/source/detail?r=7aeb0fe2819e&repo=net ***

go.net/ipv6: add missing API tests

Now the package has a code coverage of 76.1% of statements.
Also fixes flaky concurrent test.

Fixes issue 5696.

R=golang-codereviews, dave
CC=golang-codereviews
https://codereview.appspot.com/45620043

https://codereview.appspot.com/45620043/diff/220001/ipv6/multicast_test.go
File ipv6/multicast_test.go (right):

https://codereview.appspot.com/45620043/diff/220001/ipv6/multicast_test.go#ne...
ipv6/multicast_test.go:39: defer p.Close()
On 2013/12/31 10:33:22, dfc wrote:
> Why did you move this code ?

nothing, will revert.

https://codereview.appspot.com/45620043/diff/220001/ipv6/multicast_test.go#ne...
ipv6/multicast_test.go:120: defer p.Close()
On 2013/12/31 10:33:22, dfc wrote:
> same

ditto

https://codereview.appspot.com/45620043/diff/220001/ipv6/readwrite_test.go
File ipv6/readwrite_test.go (right):

https://codereview.appspot.com/45620043/diff/220001/ipv6/readwrite_test.go#ne...
ipv6/readwrite_test.go:44: if _, err := c.WriteTo(wb, dst); err != nil {
On 2013/12/31 10:33:22, dfc wrote:
> Is it necessary to check for short writes ?

Done.
Sign in to reply to this message.

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