e2e: add gRPC gateway Lease tests #9460

hexfusion · 2018-03-19T11:55:18Z

With a focus on Lease this is my initial stab at refactoring e2e grpc gateway tests. Next phase will be breaking out existing curl tests into separate files and providing full TLS coverage in prep for stable v3/.

ref: #9189, #9450

hexfusion · 2018-03-19T14:29:13Z

@gyuho PTAL re approach, thanks.

hexfusion · 2018-03-20T02:31:48Z

semaphore failure seems unrelated

Broadcast message from root@semaphore-1802
	(unknown) at 2:16 ...
The system is going down for power off NOW!

If someone could restart test, thanks.

hexfusion · 2018-03-20T02:56:57Z

hmm, maybe timeout is from the additional tests? Do we need to extend?

gyuho · 2018-03-20T16:43:29Z

e2e/v3_curl_lease_test.go

+			value:    gwKVPutLease(cx, "foo", "bar", leaseID),
+			expected: `"revision":"`,
+		},
+		{


duplicate timetolive call

gwLeaseTTLWithKeys covers bases, thanks.

gyuho · 2018-03-20T16:54:30Z

e2e/v3_curl_lease_test.go

+	return s
+}
+
+func gwLeaseNoID(cx ctlCtx, ttl int64) string {


Just combine as func gwLeaseGrant(cx ctlCtx, leaseID int64, ttl int64) string

makes sense

gyuho · 2018-03-20T16:56:42Z

e2e/v3_curl_lease_test.go

+	pb "github.com/coreos/etcd/etcdserver/etcdserverpb"
+)
+
+func TestV3CurlLeaseGrantNoTLS(t *testing.T) {


We already have a ton of TLS coverage, and now tests are taking too long.
Just keep the one with no TLS?

OK sounds good I misunderstood that you wanted to have all of these.

gyuho · 2018-03-20T16:57:55Z

e2e/v3_curl_lease_test.go

+			value:    gwLeaseWithID(cx, leaseID),
+			expected: gwLeaseIDExpected(leaseID),
+		},
+		{


remove put and range.
e2e only needs to test lease API output

understood, thanks.

gyuho · 2018-03-20T17:10:28Z

e2e/v3_curl_lease_test.go

+	}
+}
+
+func testV3CurlLeaseTTL(cx ctlCtx) {


remove timetolive and move them to testV3CurlLeaseGrant

hexfusion

Thank you for comments @gyuho, rebasing shortly.

hexfusion · 2018-03-21T00:38:19Z

e2e/v3_curl_lease_test.go

+	pb "github.com/coreos/etcd/etcdserver/etcdserverpb"
+)
+
+func TestV3CurlLeaseGrantNoTLS(t *testing.T) {


OK sounds good I misunderstood that you wanted to have all of these.

hexfusion · 2018-03-21T00:43:37Z

e2e/v3_curl_lease_test.go

+	return s
+}
+
+func gwLeaseNoID(cx ctlCtx, ttl int64) string {


makes sense

hexfusion · 2018-03-21T00:47:22Z

e2e/v3_curl_lease_test.go

+			value:    gwKVPutLease(cx, "foo", "bar", leaseID),
+			expected: `"revision":"`,
+		},
+		{


gwLeaseTTLWithKeys covers bases, thanks.

hexfusion · 2018-03-21T00:58:32Z

e2e/v3_curl_lease_test.go

+			value:    gwLeaseWithID(cx, leaseID),
+			expected: gwLeaseIDExpected(leaseID),
+		},
+		{


understood, thanks.

codecov-io · 2018-03-21T06:12:49Z

Codecov Report

Merging #9460 into master will decrease coverage by 0.3%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #9460      +/-   ##
==========================================
- Coverage   72.96%   72.65%   -0.31%     
==========================================
  Files         364      364              
  Lines       30883    30890       +7     
==========================================
- Hits        22533    22443      -90     
- Misses       6749     6829      +80     
- Partials     1601     1618      +17

Impacted Files	Coverage Δ
clientv3/integration/util.go	`66.66% <0%> (-33.34%)`	⬇️
pkg/transport/timeout_conn.go	`80% <0%> (-20%)`	⬇️
proxy/grpcproxy/register.go	`72.22% <0%> (-11.12%)`	⬇️
etcdserver/api/v3rpc/lease.go	`78.37% <0%> (-8.11%)`	⬇️
pkg/fileutil/purge.go	`73.68% <0%> (-7.9%)`	⬇️
pkg/tlsutil/tlsutil.go	`86.2% <0%> (-6.9%)`	⬇️
clientv3/namespace/watch.go	`72.72% <0%> (-6.07%)`	⬇️
etcdctl/ctlv3/command/lease_command.go	`65.34% <0%> (-5.95%)`	⬇️
clientv3/yaml/config.go	`81.25% <0%> (-5.42%)`	⬇️
proxy/grpcproxy/watcher.go	`89.79% <0%> (-4.09%)`	⬇️
... and 27 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f7714e2...8dab18a. Read the comment docs.

hexfusion · 2018-03-21T16:42:50Z

@gyuho knits should be all addressed, PTAL.

gyuho · 2018-03-22T00:55:34Z

e2e/v3_curl_lease_test.go

+
+type v3cURLTest struct {
+	endpoint string
+	value    interface{}


value should string type?

@gyuho yeah, because of the changing data types I am passing for example pb.LeaseTimeToLiveRequest then pb.LeaseKeepAliveRequest etc I found interface{} useful this generic typing. But perhaps that data flexibility is an issue in itself?

Let me revist this.

@gyuho you were right I simplified this so that it can handle a string. This is now resolved, thanks!

gyuho

thanks!

hexfusion · 2018-03-22T02:11:00Z

@gyuho looks like semaphore did not get included here in checks, is that expected?

gyuho · 2018-03-22T02:23:27Z

@hexfusion Should be safe to merge. Previous run passed.

hexfusion added the WIP - DO NOT MERGE label Mar 19, 2018

hexfusion force-pushed the e2e_watch_tests branch 3 times, most recently from 2c50ae3 to f400153 Compare March 19, 2018 13:32

hexfusion force-pushed the e2e_watch_tests branch from 2611b93 to 46979c0 Compare March 20, 2018 01:42

hexfusion changed the title ~~e2e: refactor of grpc-gateway tests.~~ e2e: add gRPC gateway Lease tests Mar 20, 2018

hexfusion removed the WIP - DO NOT MERGE label Mar 20, 2018

hexfusion force-pushed the e2e_watch_tests branch from 46979c0 to b19bd8e Compare March 20, 2018 01:58

hexfusion mentioned this pull request Mar 20, 2018

etcdserver: improve Lease http path naming for gRPC gateway #9450

Merged

gyuho suggested changes Mar 20, 2018

View reviewed changes

hexfusion commented Mar 21, 2018

View reviewed changes

hexfusion force-pushed the e2e_watch_tests branch from b19bd8e to eeaaec5 Compare March 21, 2018 01:08

gyuho reviewed Mar 22, 2018

View reviewed changes

e2e: add gRPC gateway Lease tests

8dab18a

hexfusion force-pushed the e2e_watch_tests branch from eeaaec5 to 8dab18a Compare March 22, 2018 01:13

gyuho approved these changes Mar 22, 2018

View reviewed changes

hexfusion merged commit 31de834 into etcd-io:master Mar 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

e2e: add gRPC gateway Lease tests #9460

e2e: add gRPC gateway Lease tests #9460

hexfusion commented Mar 19, 2018 •

edited

Loading

hexfusion commented Mar 19, 2018

hexfusion commented Mar 20, 2018

hexfusion commented Mar 20, 2018

gyuho Mar 20, 2018

hexfusion Mar 21, 2018

gyuho Mar 20, 2018

hexfusion Mar 21, 2018

gyuho Mar 20, 2018

hexfusion Mar 21, 2018

gyuho Mar 20, 2018

hexfusion Mar 21, 2018

gyuho Mar 20, 2018

hexfusion left a comment

hexfusion Mar 21, 2018

hexfusion Mar 21, 2018

hexfusion Mar 21, 2018

hexfusion Mar 21, 2018

codecov-io commented Mar 21, 2018 •

edited

Loading

hexfusion commented Mar 21, 2018 •

edited

Loading

gyuho Mar 22, 2018

hexfusion Mar 22, 2018

hexfusion Mar 22, 2018

hexfusion Mar 22, 2018

gyuho left a comment

hexfusion commented Mar 22, 2018

gyuho commented Mar 22, 2018

e2e: add gRPC gateway Lease tests #9460

e2e: add gRPC gateway Lease tests #9460

Conversation

hexfusion commented Mar 19, 2018 • edited Loading

hexfusion commented Mar 19, 2018

hexfusion commented Mar 20, 2018

hexfusion commented Mar 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hexfusion left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Mar 21, 2018 • edited Loading

Codecov Report

hexfusion commented Mar 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gyuho left a comment

Choose a reason for hiding this comment

hexfusion commented Mar 22, 2018

gyuho commented Mar 22, 2018

hexfusion commented Mar 19, 2018 •

edited

Loading

codecov-io commented Mar 21, 2018 •

edited

Loading

hexfusion commented Mar 21, 2018 •

edited

Loading