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

ZOOKEEPER-3269 - Add queueEvent to the Testable facade #799

Closed
wants to merge 3 commits into from

Conversation

Randgalt
Copy link
Member

@Randgalt Randgalt commented Feb 3, 2019

Add queueEvent to the Testable facade to enabled inserting events into the clients queue.

For testing and other reasons it would be very useful to add a way to inject an event into ZooKeeper's event queue. ZooKeeper already has the Testable for features such as this (low level, backdoor, testing, etc.). This queueEvent method would be particularly helpful to Apache Curator and we'd very much appreciate its inclusion.

Add queueEvent to the Testable facade to enabled inserting events into the clients queue.
Copy link
Contributor

@anmolnar anmolnar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 LGTM

import org.apache.zookeeper.ZooDefs;
import org.apache.zookeeper.ZooKeeper;
import org.apache.zookeeper.data.Stat;
import org.apache.zookeeper.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I am not sure we are preferring star import

@eolivelli
Copy link
Contributor

eolivelli commented Feb 4, 2019

LGTM
@anmolnar do we allow star imports ?

@anmolnar
Copy link
Contributor

anmolnar commented Feb 4, 2019

@eolivelli Good catch, we do not.
@Randgalt Please fix it.

@asfgit
Copy link

asfgit commented Feb 5, 2019

@Randgalt
Copy link
Member Author

Randgalt commented Feb 5, 2019

That test failure has nothing to do with this PR BTW

@eolivelli
Copy link
Contributor

Off topic:
@anmolnar @nkalmar should we disable "contrib" tests ? the 'full-build' profile runs java + contribs + cpp

@anmolnar
Copy link
Contributor

anmolnar commented Feb 5, 2019

@Randgalt use the brand new magic words:
retest this please
to kick off a new build

@eolivelli Yes please. java + cpp is enough for pull requests

@asfgit
Copy link

asfgit commented Feb 5, 2019

@eolivelli
Copy link
Contributor

@anmolnar we have new magic words..
https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute

retest maven build

@asfgit
Copy link

asfgit commented Feb 5, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build-maven/27/

Build result: ABORTED

[...truncated 86.73 KB...][INFO] Copying asm-5.0.4.jar to /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-server/target/lib/asm-5.0.4.jar[INFO] Copying kerb-admin-1.1.0.jar to /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-server/target/lib/kerb-admin-1.1.0.jar[INFO] Copying kerb-server-1.1.0.jar to /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-server/target/lib/kerb-server-1.1.0.jar[INFO] Copying kerb-identity-1.1.0.jar to /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-server/target/lib/kerb-identity-1.1.0.jar[INFO] Copying kerby-xdr-1.1.0.jar to /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-server/target/lib/kerby-xdr-1.1.0.jar[INFO] Copying kerby-config-1.1.0.jar to /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-server/target/lib/kerby-config-1.1.0.jar[INFO] Copying mockito-all-1.8.5.jar to /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-server/target/lib/mockito-all-1.8.5.jar[INFO] Copying junit-4.12.jar to /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-server/target/lib/junit-4.12.jar[INFO] Copying hamcrest-core-1.3.jar to /home/jenkins/jenkins-slave/workspace/PreCommit-ZOOKEEPER-github-pr-build-maven/zookeeper-server/target/lib/hamcrest-core-1.3.jar[WARNING] Failed to notify spy hudson.maven.Maven3Builder$JenkinsEventSpy: java.util.concurrent.ExecutionException: Invalid object ID 18 iota=25[INFO] [INFO] >>> spotbugs-maven-plugin:3.1.9:check (default-cli) > :spotbugs @ zookeeper-server >>>[INFO] [INFO] --- spotbugs-maven-plugin:3.1.9:spotbugs (spotbugs) @ zookeeper-server ---[INFO] Fork Value is true [java] SLF4J: No SLF4J providers were found. [java] SLF4J: Defaulting to no-operation (NOP) logger implementation [java] SLF4J: See http://www.slf4j.org/codes.html#noProviders for further details.Build was aborted[SpotBugs] Skipping execution of recorder since overall result is 'ABORTED' [java] channel stoppedSetting status of 0e3ab80 to FAILURE with url https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build-maven/27/ and message: 'FAILURE 'Using context: JenkinsMaven
--none----none--

@anmolnar
Copy link
Contributor

anmolnar commented Feb 6, 2019

@eolivelli I'll commit this regardless of Maven failure.

@asfgit asfgit closed this in f0f75d8 Feb 6, 2019
@anmolnar
Copy link
Contributor

anmolnar commented Feb 6, 2019

Merged to master. Thanks @Randgalt !

RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 3, 2022
Add queueEvent to the Testable facade to enabled inserting events into the clients queue.

For testing and other reasons it would be very useful to add a way to inject an event into ZooKeeper's event queue. ZooKeeper already has the Testable for features such as this (low level, backdoor, testing, etc.). This queueEvent method would be particularly helpful to Apache Curator and we'd very much appreciate its inclusion.

Author: randgalt <jordan@jordanzimmerman.com>

Reviewers: andor@apache.org

Closes apache#799 from Randgalt/ZOOKEEPER-3269 and squashes the following commits:

0e3ab80 [randgalt] ZOOKEEPER-3269 - use explicit imports
f77574b [randgalt] ZOOKEEPER-3269 remove IDE introduced spacing changes
8b5222d [randgalt] ZOOKEEPER-3269
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants