> On Nov. 10, 2014, 11:32 a.m., Robert Nettleton wrote: > > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java, > > line 654 > > <https://reviews.apache.org/r/27700/diff/2/?file=754428#file754428line654> > > > > I'm a little worried by the presence of the sleep here. > > > > It might be worth considering making this a functional test instead? > > > > Again, just concerned about the unit tests having intermittent failures > > due to concurrency problems. > > Robert Levas wrote: > We really need to sleep here so that the scheduler and executer have time > to run - which are in different threads. The idea is that a tasks a scheuled > and we are polling its status to see when it is completed.. then we make sure > its status is what was expected. I don't think there is another way around it. > > John Speidel wrote: > I agree with Bob on this and feel strongly that using sleep statements > like this result in non-deterministic tests. > I haven't really looked at the ActionScheduler to provide you a solution > but can give some genral comments on testing of this nature. > - If you can run the test in a single thread, that is always best. For > example, sometimes it is possible to call run directly. > - If a separate thread must be used, it is best to have a deterministic > way to determine if a task has completed. For example, if you have access to > the task perhaps it has state that can be queried. > - If you must use another thread and have no way to determine with > certainty that a task has completed, use a "wait for" approach. The idea is > that you have a loop that does an assertion every couple of ms. You keep > looping until the assertion holds or until you "timeout" and fail the test. > The timeout should be rather long since it is only applicable in the case of > a test failure. For the successful scenario, the test will not block for long > sine we are checking the condition in small intervals.
Nate and I had a similar issue with testing alert events. Because of the asynchronous nature of the eventing system, we would have needed to insert Thread.sleep() in a bunch of tests. Instead, we injected a different event scheduler which was hard coded to be synchronous. Maybe a similar solution can be used here where you can inject a single-threaded version of the class that blocks until its thread completes. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27700/#review60608 ----------------------------------------------------------- On Nov. 10, 2014, 9:05 a.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27700/ > ----------------------------------------------------------- > > (Updated Nov. 10, 2014, 9:05 a.m.) > > > Review request for Ambari, dilli dorai, Jonathan Hurley, John Speidel, Nate > Cole, Robert Nettleton, and Sid Wagle. > > > Bugs: AMBARI-7985 > https://issues.apache.org/jira/browse/AMBARI-7985 > > > Repository: ambari > > > Description > ------- > > Ambari currently handles client-/agent-side commands; however there is no > ability to handle server-side commands. Server-side commands should be > specified as a task in a stage and managed along with the stage. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessor.java > 1f99b4a > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionDBAccessorImpl.java > 5e879cc > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionManager.java > e2fad5f > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/ActionScheduler.java > 81fee75 > > ambari-server/src/main/java/org/apache/ambari/server/actionmanager/Stage.java > bbc5ac3 > > ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java > 52b0ba6 > > ambari-server/src/main/java/org/apache/ambari/server/orm/dao/HostRoleCommandDAO.java > 6920a9e > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/AbstractServerAction.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerAction.java > be885b5 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionExecutor.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionManager.java > 011cf06 > > ambari-server/src/main/java/org/apache/ambari/server/serveraction/ServerActionManagerImpl.java > 3a16c77 > > ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentHostEvent.java > 78590fc > > ambari-server/src/main/java/org/apache/ambari/server/state/ServiceComponentHostEventType.java > b43ac9c > > ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostServerActionEvent.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/state/svccomphost/ServiceComponentHostUpgradeEvent.java > 8b375fe > > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionDBAccessorImpl.java > 36acbc2 > > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionManager.java > 5a2c467 > > ambari-server/src/test/java/org/apache/ambari/server/actionmanager/TestActionScheduler.java > 7224924 > > ambari-server/src/test/java/org/apache/ambari/server/agent/TestHeartbeatHandler.java > 6e78b1d > > ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java > b2c023f > > ambari-server/src/test/java/org/apache/ambari/server/serveraction/MockServerAction.java > PRE-CREATION > > ambari-server/src/test/python/org/apache/ambari/server/serveraction/ServerActionExecutorTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/27700/diff/ > > > Testing > ------- > > ------------------------------------------------------------------------------- > Test set: org.apache.ambari.server.serveraction.ServerActionExecutorTest > ------------------------------------------------------------------------------- > Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 41.822 sec > > > Thanks, > > Robert Levas > >
