-
Notifications
You must be signed in to change notification settings - Fork 910
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
Fix flaky test in ctl module #653
Conversation
cc @turboFei |
Codecov Report
@@ Coverage Diff @@
## master #653 +/- ##
==========================================
+ Coverage 80.05% 80.07% +0.02%
==========================================
Files 120 120
Lines 4693 4693
Branches 568 568
==========================================
+ Hits 3757 3758 +1
Misses 611 611
+ Partials 325 324 -1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks
thanks, merged to master |
<!-- Thanks for sending a pull request! Here are some tips for you: 1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html 2. If the PR is related to an issue in https://github.com/NetEase/kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'. 3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'. --> ### _Why are the changes needed?_ <!-- Please clarify why the changes are needed. For instance, 1. If you add a feature, you can talk about the use case of it. 2. If you fix a bug, you can clarify why it is a bug. --> As `zookeeper` mentioned, the result of getChildren is no guarantee order. So it is also no guarantee order that use exists children to create new node . flaky test log: ``` - test expose zk service node to another namespace *** FAILED *** "...ceUri=localhost:1000[1];version=1.2.0;seque..." did not equal "...ceUri=localhost:1000[0];version=1.2.0;seque..." (ServiceControlCliSuite.scala:212) Analysis: "...ceUri=localhost:1000[1];version=1.2.0;seque..." -> "...ceUri=localhost:1000[0];version=1.2.0;seque..." ``` zookeeper description: ``` /** * For the given znode path return the stat and children list. * <p> * If the watch is true and the call is successful (no exception is thrown), * a watch will be left on the node with the given path. The watch will be * triggered by a successful operation that deletes the node of the given * path or creates/delete a child under the node. * <p> * The list of children returned is not sorted and no guarantee is provided * as to its natural or lexical order. * <p> * A KeeperException with error code KeeperException.NoNode will be thrown * if no node with the given path exists. * * since 3.3.0 * * param path * param watch * param stat stat of the znode designated by path * return an unordered array of children of the node with the given path * throws InterruptedException If the server transaction is interrupted. * throws KeeperException If the server signals an error with a non-zero * error code. */ public List<String> getChildren(String path, boolean watch, Stat stat) throws KeeperException, InterruptedException { return getChildren(path, watch ? watchManager.defaultWatcher : null, stat); } ``` ### _How was this patch tested?_ Pass CI. Closes #653 from ulysses-you/flaky-test-ctl. Closes #653 0be8248 [ulysses-you] fix Authored-by: ulysses-you <ulyssesyou18@gmail.com> Signed-off-by: fwang12 <fwang12@ebay.com> (cherry picked from commit 55df645) Signed-off-by: ulysses-you <ulyssesyou18@gmail.com>
thanks , also merged to branch-1.2 |
Why are the changes needed?
As
zookeeper
mentioned, the result of getChildren is no guarantee order. So it is also no guarantee order that use exists children to create new node .flaky test log:
zookeeper description:
How was this patch tested?
Pass CI.