Mailing-List: contact dev-help@ambari.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@ambari.apache.org
Content-Type: multipart/alternative;
 boundary="===============7681661838052394395=="
MIME-Version: 1.0
Subject: Re: Review Request 35074: ClusterDeadlockTest unit test fails
From: "Dmytro Shkvyra" <dshkvyra@hortonworks.com>
To: "John Speidel" <jspeidel@hortonworks.com>,
 "Jonathan Hurley" <jhurley@hortonworks.com>
Cc: "Dmytro Shkvyra" <dshkvyra@hortonworks.com>,
 "Ambari" <dev@ambari.apache.org>,
 "Vitalyi Brodetskyi" <vbrodetskyi@hortonworks.com>
Date: Tue, 09 Jun 2015 16:43:16 -0000
Message-ID: <20150609164316.27735.50648@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Dmytro Shkvyra" <noreply@reviews.apache.org>
References: <20150609154311.27736.68586@reviews.apache.org>
In-Reply-To: <20150609154311.27736.68586@reviews.apache.org>
Reply-To: "Dmytro Shkvyra" <dshkvyra@hortonworks.com>

--===============7681661838052394395==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


> 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
> 
> John Speidel wrote:
>     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.

Ok


- Dmytro


-----------------------------------------------------------
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 <onechiporenko@apache.org>
> 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
> 
>


--===============7681661838052394395==--