auth: refactor auth store test to use common setup #7234

Rushit · 2017-01-25T19:12:00Z

Refactored auth store tests to pull common setup into a method.

as per https://github.com/coreos/etcd/pull/7229/files/1b9a4d891148eca86d28ef051676f1eb5e49fa00
/cc @xiang90 @mitake

Rushit · 2017-01-25T20:07:44Z

I am not sure how refactoring test failed etcd/integration tests

https://jenkins-etcd-public.prod.coreos.systems/job/etcd-proxy/563/console

`
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xf2273a]

goroutine 124588 [running]:
panic(0x10d1260, 0xc4200100d0)
/usr/lib/go-1.7/src/runtime/panic.go:500 +0x1ae
github.com/boltdb/bolt.(*Bucket).pageNode(0xc432698398, 0x4, 0x9, 0x0)
/home/jenkins/workspace/etcd-proxy/gopath/src/github.com/boltdb/bolt/bucket.go:697 +0x33a
github.com/boltdb/bolt.(*Cursor).First(0xc4222814f0, 0x0, 0x0, 0x175bd10, 0x0, 0x0, 0x0)
/home/jenkins/workspace/etcd-proxy/gopath/src/github.com/boltdb/bolt/cursor.go:34 +0x18a
github.com/coreos/etcd/mvcc/backend.(*backend).Hash.func1(0xc432698380, 0x12d2dc8, 0xc432698380)
/home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/mvcc/backend/backend.go:159 +0x1e9
github.com/boltdb/bolt.(*DB).View(0xc424fe6000, 0xc4222815e0, 0x0, 0x0)
/home/jenkins/workspace/etcd-proxy/gopath/src/github.com/boltdb/bolt/db.go:626 +0xe2
github.com/coreos/etcd/mvcc/backend.(*backend).Hash(0xc42d329e60, 0xc42014eb10, 0xc400000000, 0x0, 0x0)
/home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/mvcc/backend/backend.go:175 +0x1b5
github.com/coreos/etcd/mvcc.(*store).Hash(0xc42c861c70, 0xc400000000, 0x0, 0x0, 0x0)
/home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/mvcc/kvstore.go:341 +0xfe
github.com/coreos/etcd/mvcc.(*watchableStore).Hash(0xc42138f440, 0x17120a0, 0xc42138f440, 0x0, 0x0)
:67 +0x69
github.com/coreos/etcd/etcdserver/api/v3rpc.(*maintenanceServer).Hash(0xc4201a22a0, 0x7f6c93e4a9d0, 0xc42603ff80, 0x175bd10, 0x0, 0x1108d20, 0x1125140)
/home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/etcdserver/api/v3rpc/maintenance.go:123 +0x6c
github.com/coreos/etcd/etcdserver/api/v3rpc.(*authMaintenanceServer).Hash(0xc424bb9540, 0x7f6c93e4a9d0, 0xc42603ff80, 0x175bd10, 0x3, 0x170ff80, 0xc430dca6c0)
/home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/etcdserver/api/v3rpc/maintenance.go:184 +0xe8
github.com/coreos/etcd/etcdserver/etcdserverpb._Maintenance_Hash_Handler.func1(0x7f6c93e4a9d0, 0xc42603ff80, 0x1159320, 0x175bd10, 0xc425389db0, 0x8, 0x1109c20, 0x1109c20)
/home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/etcdserver/etcdserverpb/rpc.pb.go:3067 +0xe9
github.com/grpc-ecosystem/go-grpc-prometheus.UnaryServerInterceptor(0x7f6c93e4a9d0, 0xc42603ff80, 0x1159320, 0x175bd10, 0xc42306aae0, 0xc42306ab00, 0xc4201ce400, 0x7f6c93f4e000, 0x0, 0xc42306aac0)
/home/jenkins/workspace/etcd-proxy/gopath/src/github.com/grpc-ecosystem/go-grpc-prometheus/server.go:29 +0xd0
github.com/coreos/etcd/etcdserver/api/v3rpc.newUnaryInterceptor.func1(0x7f6c93e4a9d0, 0xc42603ff80, 0x1159320, 0x175bd10, 0xc42306aae0, 0xc42306ab00, 0x7f6c83446940, 0x0, 0xc42603ff00, 0xc4201ce400)
/home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/etcdserver/api/v3rpc/interceptor.go:57 +0x106
github.com/coreos/etcd/etcdserver/etcdserverpb._Maintenance_Hash_Handler(0x1125140, 0xc424bb9540, 0x7f6c93e4a9d0, 0xc42603ff80, 0xc425389d60, 0xc42132d170, 0x0, 0x0, 0x0, 0xc42e094a80)
/home/jenkins/workspace/etcd-proxy/gopath/src/github.com/coreos/etcd/etcdserver/etcdserverpb/rpc.pb.go:3069 +0x1db
google.golang.org/grpc.(*Server).processUnaryRPC(0xc4357b8000, 0x1710100, 0xc420358480, 0xc430bcf770, 0xc43297b050, 0x17305c8, 0xc42603fef0, 0x0, 0x0)
/home/jenkins/workspace/etcd-proxy/gopath/src/google.golang.org/grpc/server.go:638 +0x10c8
google.golang.org/grpc.(*Server).handleStream(0xc4357b8000, 0x1710100, 0xc420358480, 0xc430bcf770, 0xc42603fef0)
/home/jenkins/workspace/etcd-proxy/gopath/src/google.golang.org/grpc/server.go:796 +0xf9d
google.golang.org/grpc.(*Server).serveStreams.func1.1(0xc4260429d0, 0xc4357b8000, 0x1710100, 0xc420358480, 0xc430bcf770)
/home/jenkins/workspace/etcd-proxy/gopath/src/google.golang.org/grpc/server.go:449 +0xb9
created by google.golang.org/grpc.(*Server).serveStreams.func1
/home/jenkins/workspace/etcd-proxy/gopath/src/google.golang.org/grpc/server.go:450 +0xbb
exit status 2
FAIL github.com/coreos/etcd/integration 381.327s
`

codecov-io · 2017-01-25T20:23:00Z

Current coverage is 64.05% (diff: 100%)

No coverage report found for master at 094be29.

Powered by Codecov. Last update 094be29...a55aa86

heyitsanthony · 2017-01-25T21:01:41Z

@Rushit it's a known issue (#6989), no worries

xiang90 · 2017-01-26T02:48:59Z

auth/store_test.go

@@ -34,32 +34,35 @@ func dummyIndexWaiter(index uint64) <-chan struct{} {
 	return ch
 }

-func TestUserAdd(t *testing.T) {
+func setupAuthStore(t *testing.T) (*authStore, func(t *testing.T)) {


let use named return here? func(t *testing.T) is very unclear.

(store *authStore, teardownfunc func(t *testing.T))

xiang90 · 2017-01-26T02:49:20Z

auth/store_test.go


 	as := NewAuthStore(b, dummyIndexWaiter)
-	ua := &pb.AuthUserAddRequest{Name: "foo"}
+	roleAddErr := enableAuthAndCreateRoot(as)


xiang90 · 2017-01-26T02:49:46Z

auth/store_test.go

-	if err != ErrUserEmpty {
-		t.Fatal(err)
+	tearDown := func(t *testing.T) {
+		func() {


why do we need this nested func()?

good point. I carried away while moving code 😄 Updating PR.

xiang90 · 2017-01-26T02:50:21Z

Minor issues. LGTM after fixing them. Defer to @mitake

Refactored tests to pull common setup into a method.

Rushit · 2017-01-26T03:09:47Z

I have pushed changes recommended by @xiang90.

xiang90 · 2017-01-26T03:14:16Z

@Rushit Looks great now. Please wait for @mitake's review. Thank you so much.

mitake · 2017-01-26T05:52:19Z

@Rushit lgtm, thanks! The failed test isn't related to the change of this PR, so merging it.

Rushit mentioned this pull request Jan 26, 2017

auth: Adding unit tests #7229

Merged

xiang90 reviewed Jan 26, 2017

View reviewed changes

auth: refactor test to use common setup

fa1cbd5

Refactored tests to pull common setup into a method.

Rushit force-pushed the store_test_refactoring branch from a55aa86 to fa1cbd5 Compare January 26, 2017 03:07

mitake merged commit 6838ac3 into etcd-io:master Jan 26, 2017

mitake mentioned this pull request Jan 26, 2017

TestLaunchDuplicateMemberShouldFail: raft log corruption? #7239

Closed

Rushit deleted the store_test_refactoring branch January 26, 2017 06:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

auth: refactor auth store test to use common setup #7234

auth: refactor auth store test to use common setup #7234

Rushit commented Jan 25, 2017

Rushit commented Jan 25, 2017 •

edited

Loading

codecov-io commented Jan 25, 2017 •

edited

Loading

heyitsanthony commented Jan 25, 2017

xiang90 Jan 26, 2017 •

edited

Loading

xiang90 Jan 26, 2017

xiang90 Jan 26, 2017

Rushit Jan 26, 2017

xiang90 commented Jan 26, 2017

Rushit commented Jan 26, 2017 •

edited

Loading

xiang90 commented Jan 26, 2017

mitake commented Jan 26, 2017

auth: refactor auth store test to use common setup #7234

auth: refactor auth store test to use common setup #7234

Conversation

Rushit commented Jan 25, 2017

Rushit commented Jan 25, 2017 • edited Loading

codecov-io commented Jan 25, 2017 • edited Loading

Current coverage is 64.05% (diff: 100%)

heyitsanthony commented Jan 25, 2017

xiang90 Jan 26, 2017 • edited Loading

Choose a reason for hiding this comment

xiang90 Jan 26, 2017

Choose a reason for hiding this comment

xiang90 Jan 26, 2017

Choose a reason for hiding this comment

Rushit Jan 26, 2017

Choose a reason for hiding this comment

xiang90 commented Jan 26, 2017

Rushit commented Jan 26, 2017 • edited Loading

xiang90 commented Jan 26, 2017

mitake commented Jan 26, 2017

Rushit commented Jan 25, 2017 •

edited

Loading

codecov-io commented Jan 25, 2017 •

edited

Loading

xiang90 Jan 26, 2017 •

edited

Loading

Rushit commented Jan 26, 2017 •

edited

Loading