No, otherwise it wouldnt pass in idea and maven. Le 19 avr. 2018 00:57, "Kenneth Knowles" <k...@google.com> a écrit :
Does the test think a thread leaked when no thread leaked? On Wed, Apr 18, 2018 at 1:51 PM Romain Manni-Bucau <rmannibu...@gmail.com> wrote: > The leaking threadq are already dumped but we dont track who created it > > Le 18 avr. 2018 22:50, "Kenneth Knowles" <k...@google.com> a écrit : > >> For this particular test, is it failing because a different test class >> leaked threads? Is there a way to make something like a testing base class >> with @Before and @After actions that would catch the leak closer to where >> it happened? A quick search found this: https://github.com/ >> elastic/elasticsearch-mapper-attachments/issues/88 but if that doesn't >> apply maybe something else does. The very best thing is probably to use a >> dynamic analysis since it is not very practical to unit test for leaks >> (thread, memory, file handle, whatever) and other global correctness >> criteria. >> >> Kenn >> >> On Wed, Apr 18, 2018 at 10:13 AM Romain Manni-Bucau < >> rmannibu...@gmail.com> wrote: >> >>> >>> >>> Le 18 avr. 2018 18:53, "Scott Wegner" <sweg...@google.com> a écrit : >>> >>> I wanted to provide some additional information about this issue and >>> Gradle test configuration. >>> >>> For context, BEAM-4088 [1] refers to a new test on PR/4965 [2] which >>> verifies that no threads are being leaked. Under Maven the test passes, but >>> under Gradle it reports leaked threads [3]. >>> >>> I'm not familiar with this test's logic, but presumably some differences >>> in test isolation are causing the failures. In Gradle, test classes are >>> executed in parallel in forked VMs, and those VMs are reused between test >>> classes. This configuration optimizes for execution time, but can break if >>> tests are not well isolated. >>> >>> When a test fails in this configuration, the first step should be to >>> understand why it fails and see if the test can be implemented with >>> better isolation. Barring that, we can update the execution parameters at >>> the project level for stronger sandboxing at the cost of longer execution >>> time. In Gradle, this is done by setting maxParallelForks [4] and forkEvery >>> [5] options on the Test task. >>> >>> >>> I dont know, first of all no hardcoded value must be in any build file >>> until required. A required case can be "fork and dont reuse the forks" >>> cause this IO is known as leaking. >>> >>> MaxFork and forEvery are about forks. >>> The test works if it runs alone in a fork until another test execution >>> leaked and still runs. >>> >>> Killing the daemon can help (and is a good practise on the CI), maybe >>> worth a try. >>> >>> We can also fork a thread for rhis particular test bit it just hides a >>> nasty bug so probably not worth it. >>> >>> >>> [1] https://issues.apache.org/jira/browse/BEAM-4088 >>> [2] https://github.com/apache/beam/pull/4965 >>> [3] https://scans.gradle.com/s/grha56432j3t2/tests/ >>> jqhvlvf72f7pg-ipde5etqqejoa?openStackTraces=WzBd >>> [4] https://docs.gradle.org/current/dsl/org.gradle.api. >>> tasks.testing.Test.html#org.gradle.api.tasks.testing.Test: >>> maxParallelForks >>> [5] https://docs.gradle.org/current/dsl/org.gradle.api. >>> tasks.testing.Test.html#org.gradle.api.tasks.testing.Test:forkEvery >>> >>> On Thu, Apr 12, 2018 at 10:03 AM Romain Manni-Bucau < >>> rmannibu...@gmail.com> wrote: >>> >>>> Seems so but is not. Most of beam tests assume they are alone in their >>>> vm when executing for a bunch of reason, if not the case a lot of side >>>> effects can happen (backend state, local cache drop, ...., uncontrolled >>>> resources and failure due to the GBKTest which creates 100M keys etc...) so >>>> you have to have one test per vm at any time. If this assumption is true my >>>> test will pass, if it is false gradle setup is wrong. >>>> >>>> Le 12 avr. 2018 18:40, "Kenneth Knowles" <k...@google.com> a écrit : >>>> >>>>> It seems that the test probably depends on some details of Maven or >>>>> our Maven configuration. If so, that's a problem with the test. It should >>>>> be able to succeed in any build system, or as a standalone JUnit main >>>>> built >>>>> from the suite, etc. >>>>> >>>>> Kenn >>>>> >>>>> On Thu, Apr 12, 2018 at 9:27 AM Romain Manni-Bucau < >>>>> rmannibu...@gmail.com> wrote: >>>>> >>>>>> on maven it is quite reliable, ran it > 10 times without any failure. >>>>>> >>>>>> I suspect (but didnt check by lack of time) gradle parallelism is >>>>>> different somehow and can lead to some flackyness here. >>>>>> >>>>>> Romain Manni-Bucau >>>>>> @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book >>>>>> >>>>>> >>>>>> 2018-04-12 18:20 GMT+02:00 Scott Wegner <sweg...@google.com>: >>>>>> > It looks like the precommit failure [1] is for a new test that was >>>>>> added. >>>>>> > Have you debugged the test to ensure it's not flaky? >>>>>> > >>>>>> > [1] >>>>>> > https://builds.apache.org/job/beam_PreCommit_Java_ >>>>>> GradleBuild/4059/testReport/junit/org.apache.beam.runners.direct/ >>>>>> ExecutorServiceParallelExecutorTest/ensureMetricsThreadDoesntLeak/ >>>>>> > >>>>>> > On Thu, Apr 12, 2018 at 5:06 AM Romain Manni-Bucau < >>>>>> rmannibu...@gmail.com> >>>>>> > wrote: >>>>>> >> >>>>>> >> Hi guys, >>>>>> >> >>>>>> >> did the gradle track changed the way test execution was done? >>>>>> >> >>>>>> >> This PR https://github.com/apache/beam/pull/4965 works very well >>>>>> with >>>>>> >> maven and sometimes doesn't pass with gradle. Think we should keep >>>>>> the >>>>>> >> previous setup which was globally reliable (I'm not speaking of >>>>>> tests >>>>>> >> which are not but of the setup). >>>>>> >> >>>>>> >> Any inputs or in progress todo i missed? >>>>>> >> >>>>>> >> Romain Manni-Bucau >>>>>> >> @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book >>>>>> > >>>>>> > -- >>>>>> > >>>>>> > >>>>>> > Got feedback? http://go/swegner-feedback >>>>>> >>>>> -- >>> >>> >>> Got feedback? http://go/swegner-feedback >>> >>> >>>
