Right. I noticed that part too and have no idea why that sleep-hack-todo has been added originally, but it seems that it doesn’t play part in this game, so I haven’t touched it.
The other waits are not sleeping for X seconds necessarily. The setting just limits the time that they can wait for an event to happen. If it happens earlier, the test goes on similarly to your suggestion. Patch: https://github.com/apache/zookeeper/pull/1345 Jira: https://issues.apache.org/jira/browse/ZOOKEEPER-3813 Thanks, Andor > On 2020. Apr 30., at 12:15, Christopher <ctubb...@apache.org> wrote: > > Looks like I replied at the same time as you. :) > Extending the timeout may work... but tests that use Thread.sleep() to > wait for an expected condition are fragile. The current code even > labels the strategy as "TODO hack". > It would be better to loop and re-check for the expected condition, > until some overall timeout for the test is reached. Thread.sleep() > should only be used to throttle the loop. > > For example, a test that follows the following pattern is better than > one that simply sleeps for 30 seconds and checks the expected > condition once: > > @Test(timeout = 30_000) > public void testForCondition() { > while (true) { > if (expectedCondition) break; > Thread.sleep(50); > } > } > > On Thu, Apr 30, 2020 at 6:08 AM Andor Molnar <an...@apache.org> wrote: >> >> Oh, I see now. >> >> SensitivityWatchEventModifier has been removed by this patch: >> https://github.com/apache/zookeeper/pull/1269 >> >> Which will make file change event notification slower and tests to break. On >> Mac. >> Looks like I only need to increase the timeout, because the polling >> mechanism happens every 10 seconds. >> >> I’ll put together a patch and continue my testing. >> >> Andor >> >> >> >> >>> On 2020. Apr 30., at 11:19, Andor Molnar <an...@apache.org> wrote: >>> >>> FileChangeWatcherTest keeps failing for me on Mac with the following error: >>> >>> [ERROR] Tests run: 5, Failures: 5, Errors: 0, Skipped: 0, Time elapsed: >>> 29.233 s <<< FAILURE! - in org.apache.zookeeper.common.FileChangeWatcherTest >>> [ERROR] >>> testCallbackWorksOnFileChanges(org.apache.zookeeper.common.FileChangeWatcherTest) >>> Time elapsed: 4.068 s <<< FAILURE! >>> java.lang.AssertionError: Wrong number of events expected:<1> but was:<0> >>> at >>> org.apache.zookeeper.common.FileChangeWatcherTest.testCallbackWorksOnFileChanges(FileChangeWatcherTest.java:92) >>> >>> >>> Andor >>> >>> >>> >>> >>>> On 2020. Apr 27., at 17:24, Enrico Olivelli <eolive...@gmail.com> wrote: >>>> >>>> +1 (binding) >>>> >>>> verified checksums, sigs >>>> run all tests on Fedora 31 + JDK8 >>>> checked rat, checkstyle, spotbugs >>>> performed basic tests using JDK8 using the staged binaries. >>>> >>>> >>>> Generally I don't like to self-vote, I have prepared the RC so I may be >>>> biased. >>>> >>>> Any other binding +1 would be very appreciated. >>>> btw we need another one (we only have me and Patrick as binding voters in >>>> this thread) >>>> >>>> Enrico >>>> >>>> >>>> Il giorno sab 25 apr 2020 alle ore 18:09 Enrico Olivelli < >>>> eolive...@gmail.com> ha scritto: >>>> >>>>> We still need a couple of binding VOTEs >>>>> Please any PMC check this candidate >>>>> >>>>> Enrico >>>>> >>>>> Il Ven 24 Apr 2020, 06:31 Patrick Hunt <ph...@apache.org> ha scritto: >>>>> >>>>>> On Thu, Apr 23, 2020 at 2:20 AM Enrico Olivelli <eolive...@gmail.com> >>>>>> wrote: >>>>>> >>>>>>> Il giorno mer 22 apr 2020 alle ore 16:14 Norbert Kalmar >>>>>>> <nkal...@cloudera.com.invalid> ha scritto: >>>>>>> >>>>>>>> Only thing I found is that the bin has netty-codec-4.1.49 license file >>>>>>>> while the jar included is 4.1.48. I think the license version has a >>>>>> typo >>>>>>> in >>>>>>>> the bugfix version. Not sure if it's a showstopper. >>>>>>>> >>>>>>> >>>>>>> I don't consider it a showstopper. >>>>>>> >>>>>>> Do you have time to send a fix please ? >>>>>>> This way if we have to roll out a new RC we can pick it up. >>>>>>> >>>>>> >>>>>> Sorry - my bad on that one. >>>>>> >>>>>> I submitted a simple PR to fix it if you want to pull into the other >>>>>> branches or have it ready if a respin is necessary: >>>>>> https://github.com/apache/zookeeper/pull/1333 >>>>>> >>>>>> Patrick >>>>>> >>>>>> >>>>>>> >>>>>>> We could anyhow update to 4.1.49.Final >>>>>>> https://netty.io/news/2020/04/22/4-1-49-Final.html >>>>>>> >>>>>>> Enrico >>>>>>> >>>>>>> >>>>>>>> >>>>>>>> Otherwise LGTM: >>>>>>>> - Signatures OK >>>>>>>> - Compared to git and 3.6.0 >>>>>>>> - Compiled both on Mac (without C client) and Linux (with C client) >>>>>>>> - Run tests (from src) and server (from src and bin tarball), connect >>>>>>> with >>>>>>>> client and run simple commands >>>>>>>> - Spotbugs and checkstyle passed >>>>>>>> >>>>>>>> Regards, >>>>>>>> Norbert >>>>>>>> >>>>>>>> On Wed, Apr 22, 2020 at 3:50 PM Szalay-Bekő Máté < >>>>>>>> szalay.beko.m...@gmail.com> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> +1 (non-binding) >>>>>>>>> >>>>>>>>> - I built the source code (-Pfull-build) on Ubuntu 18.04.3 using >>>>>>> OpenJDK >>>>>>>>> 8u242 and maven 3.6.0. >>>>>>>>> - all the unit tests passed (both Java and C-client). >>>>>>>>> - I also built and executed unit tests for zkpython >>>>>>>>> - checkstyle and spotbugs passed >>>>>>>>> - apache-rat passed >>>>>>>>> - fatjar built >>>>>>>>> - I executed a quick rolling-upgrade test from 3.5.7 to 3.6.1. >>>>>> (using >>>>>>>>> https://github.com/symat/zk-rolling-upgrade-test) >>>>>>>>> >>>>>>>>> On Tue, Apr 21, 2020 at 5:20 PM Enrico Olivelli < >>>>>> eolive...@gmail.com> >>>>>>>>> wrote: >>>>>>>>> >>>>>>>>>> This is a release candidate for 3.6.1. >>>>>>>>>> >>>>>>>>>> It is a bugfix release and it introduces a few bugfixes and new >>>>>>>> features >>>>>>>>> in >>>>>>>>>> these areas: >>>>>>>>>> - compatibility with applications built against 3.5 client >>>>>> libraries >>>>>>>>>> (restored a few non public APIs) >>>>>>>>>> - update Netty to 4.1.48.Final >>>>>>>>>> - ability to pass configuration as file in zkCli for TLS config >>>>>>>>>> - Add setKeepAlive support for NIOServerCnxn >>>>>>>>>> - Fix server side request throttling >>>>>>>>>> >>>>>>>>>> The full release notes is available at: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12310801&version=12346764 >>>>>>>>>> >>>>>>>>>> *** Please download, test and vote by April 14th 2020, 23:59 >>>>>> UTC+0. >>>>>>> *** >>>>>>>>>> >>>>>>>>>> Source files: >>>>>>>>>> https://people.apache.org/~eolivelli/zookeeper-3.6.1-candidate-1/ >>>>>>>>>> >>>>>>>>>> Maven staging repo: >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> https://repository.apache.org/content/repositories/orgapachezookeeper-1058/ >>>>>>>>>> >>>>>>>>>> The release candidate tag in git to be voted upon: release-3.6.1-1 >>>>>>>>>> https://github.com/apache/zookeeper/tree/release-3.6.1-1 >>>>>>>>>> >>>>>>>>>> ZooKeeper's KEYS file containing PGP keys we use to sign the >>>>>> release: >>>>>>>>>> https://www.apache.org/dist/zookeeper/KEYS >>>>>>>>>> >>>>>>>>>> The staging version of the website is: >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> https://people.apache.org/~eolivelli/zookeeper-3.6.1-candidate-1/website/ >>>>>>>>>> >>>>>>>>>> Should we release this candidate? >>>>>>>>>> >>>>>>>>>> Enrico Olivelli >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>> >>
