> On Nov. 10, 2014, 4:32 p.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.
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. - John ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27700/#review60608 ----------------------------------------------------------- On Nov. 10, 2014, 2:05 p.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27700/ > ----------------------------------------------------------- > > (Updated Nov. 10, 2014, 2:05 p.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 > >
