I 100% agree with you that we should not use Thread.sleep() anywhere in our tests. I’ve been actively fighting against it for years, because they caused tons of flaky tests in the past. We’re making quite good progress actually and recent tests are lot less fragile.
Andor > On 2020. Apr 30., at 12:23, Andor Molnar <an...@apache.org> wrote: > > 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 >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>> >>> >
