Fix coverage failures

[chaochn47]

Fix issue #13684

[codecov-commenter]

Codecov Report

Merging #13693 (b683aa1) into main (bba3937) will decrease coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main   #13693      +/-   ##
==========================================
- Coverage   72.77%   72.70%   -0.07%     
==========================================
  Files         465      465              
  Lines       37865    37865              
==========================================
- Hits        27556    27531      -25     
- Misses       8535     8554      +19     
- Partials     1774     1780       +6     

Flag	Coverage Δ
all	72.70% <ø> (-0.07%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
server/proxy/grpcproxy/register.go	69.76% <0.00%> (-9.31%)	⬇️
server/auth/simple_token.go	81.10% <0.00%> (-8.67%)	⬇️
client/v3/leasing/util.go	91.66% <0.00%> (-6.67%)	⬇️
server/storage/mvcc/watchable_store.go	85.14% <0.00%> (-3.99%)	⬇️
server/etcdserver/api/v3rpc/member.go	93.54% <0.00%> (-3.23%)	⬇️
client/v3/leasing/txn.go	88.09% <0.00%> (-3.18%)	⬇️
server/etcdserver/api/v3election/election.go	66.66% <0.00%> (-2.78%)	⬇️
server/etcdserver/api/rafthttp/msgappv2_codec.go	71.30% <0.00%> (-1.74%)	⬇️
client/pkg/v3/transport/listener_tls.go	50.81% <0.00%> (-1.63%)	⬇️
server/etcdserver/api/v3rpc/watch.go	86.91% <0.00%> (-1.01%)	⬇️
... and 12 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 bba3937...b683aa1. Read the comment docs.

[serathius]

LGTM
cc @spzala @ptabor @ahrtr

[ahrtr]

The change on the e2e test case looks good, but is it related to the the issue of coverage failure?

[ahrtr]

I won't ask change on this, but it would be better if we could only fix one thing in one PR next time.

[ahrtr]

The test linearizableReadTest indeed causes the coverage test failure. Thanks for the fix.
I will have a deep dive into the coverage test sometime later.

[ahrtr]

The change looks good.

But the directory change was made more than half year ago in this commit, why we do not see any issue in such a long time? cc @serathius @ptabor @spzala

[ahrtr]

LGTM overall.

[chaochn47]

Thanks @ahrtr @serathius, sorry I was rushing to the fix without explaining why.

There are two sub-problems in the current e2e test of https://github.com/etcd-io/etcd/blob/main/tests/e2e/ctl_v3_kv_no_quorum_test.go.

1. serializableReadTest and linearizableReadTest are both wrapped inside runCtlTest function. The first runCtlTest will terminate the initialized etcd test process clusters. So the second run of runCtlTest will timeout and throw retrying of unary invoker failed anyway because there is no server process.

   etcd/tests/e2e/ctl_v3_test.go

   Lines 254 to 265 in bdb13e2

   	func runCtlTest(t *testing.T, testFunc func(ctlCtx), testOfflineFunc func(ctlCtx), cx ctlCtx) {
   	defer func() {
   	if cx.envMap != nil {
   	for k := range cx.envMap {
   	os.Unsetenv(k)
   	}
   	cx.envMap = make(map[string]string)
   	}
   	if cx.epc != nil {
   	if errC := cx.epc.Close(); errC != nil {
   	t.Fatalf("error closing etcd processes (%v)", errC)
   	}

2. the test passed in e2e but failed at coverage is due to the matching predicate is empty string slice. So it skipped the etcdctl get "key" process output string matching. In e2e test suite, it passed because noOutputLineCount is 0 while in coverage test suite noOutputLineCount is 2

   etcd/tests/framework/e2e/util.go

   Lines 81 to 84 in bdb13e2

   	l := proc.LineCount()
   	if len(xs) == 0 && l != noOutputLineCount { // expect no output
   	return nil, fmt.Errorf("unexpected output from %v (got lines %q, line count %d) %v. Try EXPECT_DEBUG=TRUE", args, lines, l, l != noOutputLineCount)
   	}

Regarding #1 problem, I split them into two sub tests to resolve the issue.

Regarding #2 problem, if e2e test expects an error, it should match the exact error message from etcdctl or etcdctl_test instead of matching with the number of outputLines

[spzala]

lgtm Thanks @chaochn47