NeverENG commented on code in PR #3690:
URL: https://github.com/apache/dubbo-go/pull/3690#discussion_r3836060233


##########
.github/workflows/github-actions.yml:
##########
@@ -41,13 +41,46 @@ jobs:
     runs-on: ubuntu-latest
     timeout-minutes: 30
 
+    # Set at job level (not just on one step) so every step below - Run unit
+    # tests and Race Test alike - explicitly points config_center/zookeeper's
+    # tests at the service container. That's also what makes
+    # failOrSkipZkUnavailable (config_center/zookeeper/impl_test.go) fail
+    # those tests instead of skipping them if the server it names isn't
+    # reachable: ZK_ADDR being set here is this job asserting "a ZooKeeper
+    # must be up", so an unreachable one is this job's own regression, not
+    # something to pass over quietly.
+    env:

Review Comment:
   这个 env + service 是为了让 config_center/zookeeper 的迁移单测在默认 unit-test job 
里真正执行而不是静默 skip(对应 AlexStocks 之前的 P1 要求)。集成测试 docker 启动的 ZK 是给 dubbo-go-samples 
用的,不会跑这个包的单测。如果你觉得不该放这里,我可以把这两个测试挪到 Integration-Test job 或去掉 service。



##########
remoting/etcdv3/client_test.go:
##########
@@ -59,23 +59,58 @@ func (m *mockClientFacade) GetURL() *common.URL   { return 
m.url }
 func (m *mockClientFacade) IsAvailable() bool     { return true }
 func (m *mockClientFacade) Destroy()              {}
 
+// runOrSkipOnHang runs fn in a goroutine and waits up to timeout for it to
+// return, skipping the test with a clear diagnostic instead of hanging (or
+// asserting an outcome we can no longer guarantee) if it doesn't.
+//
+// These tests construct an etcd client against an address with nothing
+// listening, deliberately: they exercise what happens when etcd is
+// unreachable. That used to resolve in a bounded time because gost's
+// NewClient dialed with grpc.WithBlock(). dubbogo/gost@3412137 removed that
+// without bounding the synchronous keepSession call it guards (etcd
+// concurrency.NewSession, which grants a lease over RPC with no deadline
+// attached - see database/kv/etcd/v3/client.go in dubbogo/gost), so
+// NewClient can now hang indefinitely against an unreachable server
+// regardless of the timeout passed to it. That's a real upstream bug,
+// reported/fix pending at dubbogo/gost; skip here rather than either hang
+// or assert behavior the current dependency can't deliver.
+func runOrSkipOnHang(t *testing.T, timeout time.Duration, fn func()) {

Review Comment:
   这些 runOrSkipOnHang 已经在 08f43a5c 移除了:gost v1.14.5 的 #146 给 etcd keepSession 
的初始 grant 加了超时,原来会挂 10 分钟的测试现在正常在超时时间内失败,不再需要这个兜底。



##########
go.mod:
##########
@@ -64,10 +65,10 @@ require (
        go.yaml.in/yaml/v4 v4.0.0-rc.6
        golang.org/x/net v0.56.0
        golang.org/x/sync v0.21.0
-       golang.org/x/tools v0.47.0
        google.golang.org/grpc v1.64.1
        google.golang.org/protobuf v1.34.2
        gopkg.in/natefinch/lumberjack.v2 v2.2.1
+       gopkg.in/yaml.v3 v3.0.1

Review Comment:
   已修复:tools/benchmark/client/main.go 改用 go.yaml.in/yaml/v4,go mod tidy 后 
gopkg.in/yaml.v3 回到 indirect,和 #3586 保持一致(commit 91c99071)。



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to