You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2020/05/29 17:15:53 UTC
[GitHub] [zookeeper] Randgalt opened a new pull request #1369: ZOOKEEPER-3831 - add a compatibility test module
Randgalt opened a new pull request #1369:
URL: https://github.com/apache/zookeeper/pull/1369
ZooKeeper has many external libraries that are dependent on staying compatible. For example, Apache Curator. This new module will be used to add compatibility tests for any third party library, tool, etc. that we want to ensure maintains compatibility. The module is configured with Maven not to install/deploy so it's only for testing purposes. An initial test using Apache Curator is added.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] Randgalt commented on a change in pull request #1369: ZOOKEEPER-3831 - add a compatibility test module
Posted by GitBox <gi...@apache.org>.
Randgalt commented on a change in pull request #1369:
URL: https://github.com/apache/zookeeper/pull/1369#discussion_r436266558
##########
File path: pom.xml
##########
@@ -64,6 +64,7 @@
<module>zookeeper-client</module>
<module>zookeeper-recipes</module>
<module>zookeeper-assembly</module>
+ <module>zookeeper-compatibility-tests</module>
Review comment:
I guess it could serve to make sure there are no dependency incompatibilities between components being tested.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] symat commented on pull request #1369: ZOOKEEPER-3831 - add a compatibility test module
Posted by GitBox <gi...@apache.org>.
symat commented on pull request #1369:
URL: https://github.com/apache/zookeeper/pull/1369#issuecomment-640594574
I quickly checked the PR also on branch-3.6, the code compiled and the TestApacheCuratorCompatibility succeeded for me. I think this approach / test is important for the future 3.6 releases too, so I pushed the PR also on branch-3.6.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on pull request #1369: ZOOKEEPER-3831 - add a compatibility test module
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #1369:
URL: https://github.com/apache/zookeeper/pull/1369#issuecomment-636306613
This is a known issue.
I had already reported to the ML.
I am reviewing the PR.
I give my feedback soon.
I can anticipate that I would prefer to add an additional Maven module 'curator5'
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on a change in pull request #1369: ZOOKEEPER-3831 - add a compatibility test module
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #1369:
URL: https://github.com/apache/zookeeper/pull/1369#discussion_r436257294
##########
File path: pom.xml
##########
@@ -64,6 +64,7 @@
<module>zookeeper-client</module>
<module>zookeeper-recipes</module>
<module>zookeeper-assembly</module>
+ <module>zookeeper-compatibility-tests</module>
Review comment:
It is good to have this top level module.
I feel we will have more modules of this type in the future.
So I would like to see a separate module for curator
Like the Zookeeper clients module and the c client
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] Randgalt commented on a change in pull request #1369: ZOOKEEPER-3831 - add a compatibility test module
Posted by GitBox <gi...@apache.org>.
Randgalt commented on a change in pull request #1369:
URL: https://github.com/apache/zookeeper/pull/1369#discussion_r436265440
##########
File path: pom.xml
##########
@@ -64,6 +64,7 @@
<module>zookeeper-client</module>
<module>zookeeper-recipes</module>
<module>zookeeper-assembly</module>
+ <module>zookeeper-compatibility-tests</module>
Review comment:
So you want a 3rd level module? Something like:
```
root
zookeeper-compatibility-tests
zookeeper-compatibility-test-curator
```
What purpose, then, does the parent `zookeeper-compatibility-tests` module serve?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] Randgalt commented on pull request #1369: ZOOKEEPER-3831 - add a compatibility test module
Posted by GitBox <gi...@apache.org>.
Randgalt commented on pull request #1369:
URL: https://github.com/apache/zookeeper/pull/1369#issuecomment-639658433
> Can you please create an additional Maven submodule for Curator itself
@eolivelli what do you mean? I could rename this module if you want. This module only has Curator in it. It would be empty without it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on a change in pull request #1369: ZOOKEEPER-3831 - add a compatibility test module
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #1369:
URL: https://github.com/apache/zookeeper/pull/1369#discussion_r436265931
##########
File path: pom.xml
##########
@@ -64,6 +64,7 @@
<module>zookeeper-client</module>
<module>zookeeper-recipes</module>
<module>zookeeper-assembly</module>
+ <module>zookeeper-compatibility-tests</module>
Review comment:
It will be container of future compatibility modules like yours.
We won't have only Curator one day.
Doesn't it make sense to you?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on a change in pull request #1369: ZOOKEEPER-3831 - add a compatibility test module
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #1369:
URL: https://github.com/apache/zookeeper/pull/1369#discussion_r436257294
##########
File path: pom.xml
##########
@@ -64,6 +64,7 @@
<module>zookeeper-client</module>
<module>zookeeper-recipes</module>
<module>zookeeper-assembly</module>
+ <module>zookeeper-compatibility-tests</module>
Review comment:
It is good to have this top level module.
I feel we will have more modules of this type in the future.
So I would like to see a separate module for curator
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] Randgalt commented on pull request #1369: ZOOKEEPER-3831 - add a compatibility test module
Posted by GitBox <gi...@apache.org>.
Randgalt commented on pull request #1369:
URL: https://github.com/apache/zookeeper/pull/1369#issuecomment-636298598
Interesting - the test passes but it generates an exception somewhere on exit:
```
log4j:WARN No appenders could be found for logger (org.apache.zookeeper.server.quorum.QuorumPeerConfig).
log4j:WARN Please initialize the log4j system properly.
log4j:WARN See http://logging.apache.org/log4j/1.2/faq.html#noconfig for more info.
java.lang.NullPointerException
at org.apache.zookeeper.server.persistence.FileTxnSnapLog.fastForwardFromEdits(FileTxnSnapLog.java:321)
at org.apache.zookeeper.server.ZKDatabase.fastForwardDataBase(ZKDatabase.java:300)
at org.apache.zookeeper.server.ZooKeeperServer.shutdown(ZooKeeperServer.java:827)
at org.apache.zookeeper.server.ZooKeeperServer.shutdown(ZooKeeperServer.java:778)
at org.apache.zookeeper.server.NIOServerCnxnFactory.shutdown(NIOServerCnxnFactory.java:912)
at org.apache.curator.test.TestingZooKeeperMain.close(TestingZooKeeperMain.java:186)
at org.apache.curator.test.TestingZooKeeperServer.stop(TestingZooKeeperServer.java:118)
at org.apache.curator.test.TestingZooKeeperServer.close(TestingZooKeeperServer.java:130)
at org.apache.curator.test.TestingServer.close(TestingServer.java:178)
at org.apache.zookeeper.compatibility.TestApacheCuratorCompatibility.testBasicUsageOfApisAndRecipes(TestApacheCuratorCompatibility.java:64)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:230)
at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:58)
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] eolivelli commented on a change in pull request #1369: ZOOKEEPER-3831 - add a compatibility test module
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #1369:
URL: https://github.com/apache/zookeeper/pull/1369#discussion_r435700587
##########
File path: zookeeper-compatibility-tests/src/test/java/org/apache/zookeeper/compatibility/TestApacheCuratorCompatibility.java
##########
@@ -0,0 +1,77 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.compatibility;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.CuratorFrameworkFactory;
+import org.apache.curator.framework.recipes.cache.CuratorCache;
+import org.apache.curator.retry.RetryOneTime;
+import org.apache.curator.test.TestingServer;
+import org.junit.Test;
+
+/**
+ * Make sure minimal Apache Curator APIs work correctly. As it's a widely used ZooKeeper
+ * client library we should not break it.
+ */
+public class TestApacheCuratorCompatibility {
+ private static final int TIMEOUT_MS = 5000;
+
+ @Test
+ public void testBasicUsageOfApisAndRecipes() throws Exception {
+ try (TestingServer server = new TestingServer()) {
+ RetryOneTime retryPolicy = new RetryOneTime(1);
+ try (CuratorFramework client = CuratorFrameworkFactory.newClient(server.getConnectString(), retryPolicy)) {
+ try (CuratorCache cache = CuratorCache.build(client, "/base/path")) {
+ client.start();
+ cache.start();
+
+ BlockingQueue<String> paths = new LinkedBlockingQueue<>();
+ cache.listenable().addListener((dummy1, dummy2, data) -> paths.add(data.getPath()));
+
+ client.create().creatingParentsIfNeeded().forPath("/base/path/1");
+ client.create().creatingParentsIfNeeded().forPath("/base/path/2");
+ client.create().creatingParentsIfNeeded().forPath("/base/path/1/a");
+ client.create().creatingParentsIfNeeded().forPath("/base/path/2/a");
+
+ assertEquals("/base/path", poll(paths));
+ assertEquals("/base/path/1", poll(paths));
+ assertEquals("/base/path/2", poll(paths));
+ assertEquals("/base/path/1/a", poll(paths));
+ assertEquals("/base/path/2/a", poll(paths));
+ }
+ }
+ }
+ }
+
+ private String poll(BlockingQueue<String> queue) {
Review comment:
static?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] asfgit closed pull request #1369: ZOOKEEPER-3831 - add a compatibility test module
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1369:
URL: https://github.com/apache/zookeeper/pull/1369
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zookeeper] Randgalt commented on a change in pull request #1369: ZOOKEEPER-3831 - add a compatibility test module
Posted by GitBox <gi...@apache.org>.
Randgalt commented on a change in pull request #1369:
URL: https://github.com/apache/zookeeper/pull/1369#discussion_r436265440
##########
File path: pom.xml
##########
@@ -64,6 +64,7 @@
<module>zookeeper-client</module>
<module>zookeeper-recipes</module>
<module>zookeeper-assembly</module>
+ <module>zookeeper-compatibility-tests</module>
Review comment:
So you want a 3rd level module? Something like:
```
root
zookeeper-compatibility-tests
zookeeper-compatibility-test-curator
```
What purpose, then, does the parent `zookeeper-compatibility-tests` module serve?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org