Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TestTxnPanics need to cancel testcase from sending error to error channel #15666

Closed
Mskxn opened this issue Apr 7, 2023 · 4 comments
Closed

Comments

@Mskxn
Copy link
Contributor

Mskxn commented Apr 7, 2023

What happened?

This test has 6 testcases and run them one by one, with a select stmt to handle the timeout, but timeout can occurs no because the testcase is not panic, but is blocking for some reason.When this occurs, the errors in errc is not in the order as expected. Can we cancel the errc <- s.(string) in df when timeout?

etcd/client/v3/txn_test.go

Lines 95 to 105 in 0bdc660

for i, tt := range tests {
go tt.f()
select {
case err := <-errc:
if err != tt.err {
t.Errorf("#%d: got %s, wanted %s", i, err, tt.err)
}
case <-time.After(time.Second):
t.Errorf("#%d: did not panic, wanted panic %s", i, tt.err)
}
}

What did you expect to happen?

If the testcase is failed because of timeout, do not send error to errc, so the remaining testcases can work as expected.

How can we reproduce it (as minimally and precisely as possible)?

Add some delay to any of these testcases to make the timeout error occurs.

Anything else we need to know?

No response

Etcd version (please run commands below)

Etcd configuration (command line flags or environment variables)

Etcd debug information (please run commands below, feel free to obfuscate the IP address or FQDN in the output)

Relevant log output

bug report:

txn_test.go:103: #0: did not panic, wanted panic cannot call If twice!
txn_test.go:103: #1: did not panic, wanted panic cannot call If after Then!
txn_test.go:103: #2: did not panic, wanted panic cannot call If after Else!
txn_test.go:103: #3: did not panic, wanted panic cannot call Then twice!
txn_test.go:103: #4: did not panic, wanted panic cannot call Then after Else!
txn_test.go:100: #5: got cannot call If twice!, wanted cannot call Else twice!
@Mskxn Mskxn added the type/bug label Apr 7, 2023
@lavacat
Copy link

lavacat commented Apr 7, 2023

Tried adding a delay and got failure as expected. @Mskxn am I reproducing not as you've intended?

make test-unit
...
? go.etcd.io/etcd/client/pkg/v3/verify [no test files]
% (cd client/internal/v2 && 'env' 'ETCD_VERIFY=all' 'go' 'test' './...' '-short' '-timeout=3m' '--race')
ok go.etcd.io/etcd/client/v2 0.115s
% (cd client/v3 && 'env' 'ETCD_VERIFY=all' 'go' 'test' './...' '-short' '-timeout=3m' '--race')
--- FAIL: TestTxnPanics (1.00s)
txn_test.go:104: #2: did not panic, wanted panic cannot call If after Else!
FAIL
FAIL go.etcd.io/etcd/client/v3 4.374s
ok go.etcd.io/etcd/client/v3/clientv3util 0.054s [no tests to run]
....

--- a/client/v3/txn_test.go
+++ b/client/v3/txn_test.go
@@ -61,6 +61,7 @@ func TestTxnPanics(t *testing.T) {
                {
                        f: func() {
                                defer df()
+                               time.Sleep(2 * time.Second)
                                kv.Txn(context.TODO()).Else(op).If(cmp)
                        },

@Mskxn
Copy link
Contributor Author

Mskxn commented Apr 8, 2023

Sorry for my too-brief description, I think the failed repro it caused by the 2-second delay, which is too long, so other testcases are finished and the test routine just exits. A 1-second delay can repro it with a higher possibility.

You can also delay some of them to gain a larger race window. For example, delay case 0 for 1.5 seconds and case 1 for 1 second, it is more stable to repro. In my case, it is :

=== RUN   TestTxnPanics
    txn_test.go:105: #0: did not panic, wanted panic cannot call If twice!
    txn_test.go:102: #1: got cannot call If twice!, wanted cannot call If after Then!
--- FAIL: TestTxnPanics (1.51s)

FAIL

The second error is not from the second case, instead, it is the late error from the first

@lavacat
Copy link

lavacat commented Apr 8, 2023

@Mskxn thanks I was able to reproduce with 1s delay.

=== RUN   TestTxnPanics
    txn_test.go:104: #2: did not panic, wanted panic cannot call If after Else!
    txn_test.go:101: #3: got cannot call If after Else!, wanted cannot call Then twice!
--- FAIL: TestTxnPanics (1.01s)

Do you want to submit a fix? Seams like you have a solution in mind.

@lavacat
Copy link

lavacat commented Apr 11, 2023

please close the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants