Copilot commented on code in PR #3690:
URL: https://github.com/apache/dubbo-go/pull/3690#discussion_r3829266181
##########
config_center/zookeeper/impl_test.go:
##########
@@ -66,11 +66,11 @@ func TestGetPath(t *testing.T) {
}
func TestPublishAndRemoveConfigWithMockZk(t *testing.T) {
- cluster, client, _, err := gxzookeeper.NewMockZookeeperClient("test",
5e9)
+ client, _, err := gxzookeeper.NewZookeeperClientFromEnv("test", 5e9)
if err != nil {
t.Skipf("skip mock zk setup: %v", err)
}
Review Comment:
This test is no longer using a mock ZooKeeper client
(`NewZookeeperClientFromEnv`), but the function name and skip message still say
"MockZk" / "mock zk". Update the test name and `t.Skipf` message to reflect
that it runs against a real ZooKeeper from env/defaults.
This issue also appears on line 103 of the same file.
##########
config_center/zookeeper/impl_test.go:
##########
@@ -66,11 +66,11 @@ func TestGetPath(t *testing.T) {
}
func TestPublishAndRemoveConfigWithMockZk(t *testing.T) {
- cluster, client, _, err := gxzookeeper.NewMockZookeeperClient("test",
5e9)
+ client, _, err := gxzookeeper.NewZookeeperClientFromEnv("test", 5e9)
if err != nil {
t.Skipf("skip mock zk setup: %v", err)
}
- defer cluster.Stop()
+ defer client.Close()
Review Comment:
Now that this test can run against a real ZooKeeper instance (from
env/default), it writes under the fixed root `/dubbo/config`. That can pollute
or interfere with an existing local ZK used for real workloads. Use a
test-specific root path (e.g. include the test name / random suffix) and/or
ensure all created nodes are cleaned up via `t.Cleanup` to keep the test
isolated.
This issue also appears on line 104 of the same file.
##########
go.mod:
##########
@@ -148,3 +148,10 @@ require (
gopkg.in/ini.v1 v1.66.2 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
)
+
+// TODO(#3459): temporary local replace while dubbogo/gost's migration off the
+// forked github.com/dubbogo/go-zookeeper package (see feat/upstream-zk branch
+// of https://github.com/dubbogo/gost) is validated here. Remove this once a
+// released gost version with the upstream github.com/go-zookeeper/zk
+// dependency is published, and bump the `require` above accordingly.
+replace github.com/dubbogo/gost => github.com/NeverENG/gost
v0.0.0-20260821085803-e11b407c6b6a
Review Comment:
The new `replace` points `github.com/dubbogo/gost` to
`github.com/NeverENG/gost`, which is a supply-chain risk (untrusted fork) and
can break reproducible builds for downstream users. Prefer replacing to a
specific commit in the official `github.com/dubbogo/gost` repo (or a released
version) while the migration is pending, and ensure this `replace` is removed
before merge.
--
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]
