> On June 9, 2015, 3:43 p.m., John Speidel wrote:
> > ambari-server/src/test/java/org/apache/ambari/server/testing/DeadlockedThreadsTest.java,
> >  line 38
> > <https://reviews.apache.org/r/35074/diff/3/?file=981429#file981429line38>
> >
> >     Since DeadlockWarningThread is a test itself, I would prefer that you 
> > remove the @Test annotations from this class so that we don't have a 
> > lengthy test that just tests another test.  I appreciate that you did write 
> > this class but I don't want to have to wait for it when I am trying to 
> > merge.
> 
> Dmytro Shkvyra wrote:
>     Erm, seems to we have misundertand. DeadlockedThreadsTest is a test of 
> DeadlockWarningThread and DeadlockWarningThread have no @Test() annotation
> 
> Dmytro Shkvyra wrote:
>     Please clarify, should I remove DeadlockedThreadsTest completely?
> 
> John Speidel wrote:
>     let me look again, one minute

The point that I was trying to make was that since DeadlockWarningThread is a 
test only class, we now have a test for a test.  I am glad that you wrote this 
test but since the time that it takes to run isn't trivial, and it isn't 
testing any productioon code,  I would prefer if you just removed the @Test 
annotations so that the test isn't run automatically as part of the unit tests. 
 Leaving the test class without the annotations could be useful if we have 
issues with the test failing in the future.  If you disagree or I still seem 
confused I am available to discuss over skype.


- John


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35074/#review87203
-----------------------------------------------------------


On June 9, 2015, 12:58 p.m., Vitalyi Brodetskyi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35074/
> -----------------------------------------------------------
> 
> (Updated June 9, 2015, 12:58 p.m.)
> 
> 
> Review request for Ambari, Jonathan Hurley and John Speidel.
> 
> 
> Bugs: AMBARI-11692
>     https://issues.apache.org/jira/browse/AMBARI-11692
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Fails on 2 ubuntu workstations
> 
> {noformat}
> 
> Tests in error:
>   
> testDeadlockWhileRestartingComponents(org.apache.ambari.server.state.cluster.ClusterDeadlockTest):
>  test timed out after 75000 milliseconds
> 
> Tests run: 3016, Failures: 0, Errors: 1, Skipped: 21
> 
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO]
> [INFO] Ambari Views ...................................... SUCCESS [4.863s]
> [INFO] Ambari Server ..................................... FAILURE 
> [1:49:50.358s]
> [INFO] 
> ------------------------------------------------------------------------
> [INFO] BUILD FAILURE
> 
> {noformat}
> 
> {noformat}
> java.lang.Exception: test timed out after 75000 milliseconds
>       at java.lang.Object.wait(Native Method)
>       at java.lang.Thread.join(Thread.java:1281)
>       at java.lang.Thread.join(Thread.java:1355)
>       at 
> org.apache.ambari.server.state.cluster.ClusterDeadlockTest.testDeadlockWhileRestartingComponents(ClusterDeadlockTest.java:241)
>       at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>       at 
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
>       at 
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
>       at 
> org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:45)
>       at 
> org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:15)
>       at 
> org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:42)
>       at 
> org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:20)
>       at 
> org.junit.internal.runners.statements.FailOnTimeout$StatementThread.run(FailOnTimeout.java:62)
> 
>       at 
> org.apache.ambari.server.state.svccomphost.ServiceComponentHostImpl.getDesiredStateEntity(ServiceComponentHostImpl.java:1555)
>       at 
> org.apache.ambari.server.state.svccomphost.ServiceComponentHostImpl.isRestartRequired(ServiceComponentHostImpl.java:1478)
>       at 
> org.apache.ambari.server.state.ConfigHelper.calculateIsStaleConfigs(ConfigHelper.java:927)
>       at 
> org.apache.ambari.server.state.ConfigHelper.isStaleConfigs(ConfigHelper.java:376)
>       at 
> org.apache.ambari.server.state.cluster.ClusterImpl.getClusterHealthReport(ClusterImpl.java:2580)
> {noformat}
> 
> Reproduced in trunk, last commit
> {noformat}
> commit a52eb51d1af0edc9273a947535a2a36886e625da
> Author: Oleg Nechiporenko <[email protected]>
> Date:   Thu May 28 18:02:28 2015 +0300
> 
>     AMBARI-11484. Configs: when doing override, it's hard to find config 
> override (onechiporenko)
> 
> {noformat}
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterDeadlockTest.java
>  2f064ab 
>   
> ambari-server/src/test/java/org/apache/ambari/server/testing/DeadlockWarningThread.java
>  PRE-CREATION 
>   
> ambari-server/src/test/java/org/apache/ambari/server/testing/DeadlockedThreadsTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/35074/diff/
> 
> 
> Testing
> -------
> 
> mvn clean test
> 
> 
> Thanks,
> 
> Vitalyi Brodetskyi
> 
>

Reply via email to