Uploaded image for project: 'Metron'
  1. Metron
  2. METRON-1048

Intermittent Test Failure for SimpleHBaseEnrichmentWriterLoggingTest

    Details

    • Type: Bug
    • Status: Done
    • Priority: Major
    • Resolution: Done
    • Affects Version/s: 0.4.0
    • Fix Version/s: 0.4.1
    • Labels:
      None

      Description

      This unit test failure seems to occur sporadically. The same test passed after an inconsequential change forced a re-run of the tests.

      Tests run: 6, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 11.596 sec <<< FAILURE! - in org.apache.metron.writers.SimpleHBaseEnrichmentWriterLoggingTest
      shouldWarnOnMissingConfig(org.apache.metron.writers.SimpleHBaseEnrichmentWriterLoggingTest)  Time elapsed: 0.047 sec  <<< FAILURE!
      org.mockito.exceptions.verification.TooManyActualInvocations: 
      logger.warn(
          "No config object found for key: '{}'",
          "shew.table"
      );
      Wanted 1 time:
      -> at org.apache.metron.writers.SimpleHBaseEnrichmentWriterLoggingTest.shouldWarnOnMissingConfig(SimpleHBaseEnrichmentWriterLoggingTest.java:77)
      But was 2 times. Undesired invocation:
      -> at org.apache.metron.enrichment.writer.SimpleHbaseEnrichmentWriter$Configurations.get(SimpleHbaseEnrichmentWriter.java:72)
      
      	at org.apache.metron.writers.SimpleHBaseEnrichmentWriterLoggingTest.shouldWarnOnMissingConfig(SimpleHBaseEnrichmentWriterLoggingTest.java:77)
      	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
      	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
      	at java.lang.reflect.Method.invoke(Method.java:498)
      	at org.junit.internal.runners.TestMethod.invoke(TestMethod.java:68)
      	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.runTestMethod(PowerMockJUnit44RunnerDelegateImpl.java:316)
      	at org.junit.internal.runners.MethodRoadie$2.run(MethodRoadie.java:89)
      	at org.junit.internal.runners.MethodRoadie.runBeforesThenTestThenAfters(MethodRoadie.java:97)
      	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.executeTest(PowerMockJUnit44RunnerDelegateImpl.java:300)
      	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.executeTestInSuper(PowerMockJUnit47RunnerDelegateImpl.java:131)
      	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.access$100(PowerMockJUnit47RunnerDelegateImpl.java:59)
      	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner$TestExecutorStatement.evaluate(PowerMockJUnit47RunnerDelegateImpl.java:147)
      	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.evaluateStatement(PowerMockJUnit47RunnerDelegateImpl.java:107)
      	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.executeTest(PowerMockJUnit47RunnerDelegateImpl.java:82)
      	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.runBeforesThenTestThenAfters(PowerMockJUnit44RunnerDelegateImpl.java:288)
      	at org.junit.internal.runners.MethodRoadie.runTest(MethodRoadie.java:87)
      	at org.junit.internal.runners.MethodRoadie.run(MethodRoadie.java:50)
      	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.invokeTestMethod(PowerMockJUnit44RunnerDelegateImpl.java:208)
      	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.runMethods(PowerMockJUnit44RunnerDelegateImpl.java:147)
      	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$1.run(PowerMockJUnit44RunnerDelegateImpl.java:121)
      	at org.junit.internal.runners.ClassRoadie.runUnprotected(ClassRoadie.java:34)
      	at org.junit.internal.runners.ClassRoadie.runProtected(ClassRoadie.java:44)
      	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.run(PowerMockJUnit44RunnerDelegateImpl.java:123)
      	at org.powermock.modules.junit4.common.internal.impl.JUnit4TestSuiteChunkerImpl.run(JUnit4TestSuiteChunkerImpl.java:121)
      	at org.powermock.modules.junit4.common.internal.impl.AbstractCommonPowerMockRunner.run(AbstractCommonPowerMockRunner.java:53)
      	at org.powermock.modules.junit4.PowerMockRunner.run(PowerMockRunner.java:59)
      	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:283)
      	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:173)
      	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:153)
      	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:128)
      	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:203)
      	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:155)
      	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:103)
      

      https://travis-ci.org/apache/metron/builds/253739859

        Issue Links

          Activity

          Hide
          nickwallen Nick Allen added a comment -

          Looks like we have another example.

          https://travis-ci.org/apache/metron/builds/254682230

          Show
          nickwallen Nick Allen added a comment - Looks like we have another example. https://travis-ci.org/apache/metron/builds/254682230
          Hide
          nickwallen Nick Allen added a comment -
          Show
          nickwallen Nick Allen added a comment - And another example. https://travis-ci.org/nickwallen/metron/builds/254892939
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user nickwallen opened a pull request:

          https://github.com/apache/metron/pull/657

          METRON-1048 Intermittent Test Failure for SimpleHBaseEnrichmentWriterLoggingTest

              1. Problem

          This unit test failure seems to occur sporadically. The same test passed after an inconsequential change forced a re-run of the tests.

          ```
          Tests run: 6, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 11.596 sec <<< FAILURE! - in org.apache.metron.writers.SimpleHBaseEnrichmentWriterLoggingTest
          shouldWarnOnMissingConfig(org.apache.metron.writers.SimpleHBaseEnrichmentWriterLoggingTest) Time elapsed: 0.047 sec <<< FAILURE!
          org.mockito.exceptions.verification.TooManyActualInvocations:
          logger.warn(
          "No config object found for key: '{}'",
          "shew.table"
          );
          Wanted 1 time:
          -> at org.apache.metron.writers.SimpleHBaseEnrichmentWriterLoggingTest.shouldWarnOnMissingConfig(SimpleHBaseEnrichmentWriterLoggingTest.java:77)
          But was 2 times. Undesired invocation:
          -> at org.apache.metron.enrichment.writer.SimpleHbaseEnrichmentWriter$Configurations.get(SimpleHbaseEnrichmentWriter.java:72)

          at org.apache.metron.writers.SimpleHBaseEnrichmentWriterLoggingTest.shouldWarnOnMissingConfig(SimpleHBaseEnrichmentWriterLoggingTest.java:77)
          at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
          at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
          at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
          at java.lang.reflect.Method.invoke(Method.java:498)
          at org.junit.internal.runners.TestMethod.invoke(TestMethod.java:68)
          at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.runTestMethod(PowerMockJUnit44RunnerDelegateImpl.java:316)
          at org.junit.internal.runners.MethodRoadie$2.run(MethodRoadie.java:89)
          at org.junit.internal.runners.MethodRoadie.runBeforesThenTestThenAfters(MethodRoadie.java:97)
          at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.executeTest(PowerMockJUnit44RunnerDelegateImpl.java:300)
          at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.executeTestInSuper(PowerMockJUnit47RunnerDelegateImpl.java:131)
          at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.access$100(PowerMockJUnit47RunnerDelegateImpl.java:59)
          at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner$TestExecutorStatement.evaluate(PowerMockJUnit47RunnerDelegateImpl.java:147)
          at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.evaluateStatement(PowerMockJUnit47RunnerDelegateImpl.java:107)
          at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.executeTest(PowerMockJUnit47RunnerDelegateImpl.java:82)
          at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.runBeforesThenTestThenAfters(PowerMockJUnit44RunnerDelegateImpl.java:288)
          at org.junit.internal.runners.MethodRoadie.runTest(MethodRoadie.java:87)
          at org.junit.internal.runners.MethodRoadie.run(MethodRoadie.java:50)
          at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.invokeTestMethod(PowerMockJUnit44RunnerDelegateImpl.java:208)
          at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.runMethods(PowerMockJUnit44RunnerDelegateImpl.java:147)
          at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$1.run(PowerMockJUnit44RunnerDelegateImpl.java:121)
          at org.junit.internal.runners.ClassRoadie.runUnprotected(ClassRoadie.java:34)
          at org.junit.internal.runners.ClassRoadie.runProtected(ClassRoadie.java:44)
          at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.run(PowerMockJUnit44RunnerDelegateImpl.java:123)
          at org.powermock.modules.junit4.common.internal.impl.JUnit4TestSuiteChunkerImpl.run(JUnit4TestSuiteChunkerImpl.java:121)
          at org.powermock.modules.junit4.common.internal.impl.AbstractCommonPowerMockRunner.run(AbstractCommonPowerMockRunner.java:53)
          at org.powermock.modules.junit4.PowerMockRunner.run(PowerMockRunner.java:59)
          at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:283)
          at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:173)
          at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:153)
          at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:128)
          at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:203)
          at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:155)
          at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:103)
          ```

              1. Solution

          The test was expecting the call to occur only once, when in-fact it could occur a few times. This is why it would fail sporadically. This doesn't impact the validity of the test.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/nickwallen/metron METRON-1048

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/metron/pull/657.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #657


          commit a3b9369451ccd859a12983797088a7ed85ccc4d0
          Author: Nick Allen <nick@nickallen.org>
          Date: 2017-07-18T16:39:06Z

          METRON-1048 Intermittent Test Failure for SimpleHBaseEnrichmentWriterLoggingTest


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user nickwallen opened a pull request: https://github.com/apache/metron/pull/657 METRON-1048 Intermittent Test Failure for SimpleHBaseEnrichmentWriterLoggingTest Problem This unit test failure seems to occur sporadically. The same test passed after an inconsequential change forced a re-run of the tests. ``` Tests run: 6, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 11.596 sec <<< FAILURE! - in org.apache.metron.writers.SimpleHBaseEnrichmentWriterLoggingTest shouldWarnOnMissingConfig(org.apache.metron.writers.SimpleHBaseEnrichmentWriterLoggingTest) Time elapsed: 0.047 sec <<< FAILURE! org.mockito.exceptions.verification.TooManyActualInvocations: logger.warn( "No config object found for key: '{}'", "shew.table" ); Wanted 1 time: -> at org.apache.metron.writers.SimpleHBaseEnrichmentWriterLoggingTest.shouldWarnOnMissingConfig(SimpleHBaseEnrichmentWriterLoggingTest.java:77) But was 2 times. Undesired invocation: -> at org.apache.metron.enrichment.writer.SimpleHbaseEnrichmentWriter$Configurations.get(SimpleHbaseEnrichmentWriter.java:72) at org.apache.metron.writers.SimpleHBaseEnrichmentWriterLoggingTest.shouldWarnOnMissingConfig(SimpleHBaseEnrichmentWriterLoggingTest.java:77) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.junit.internal.runners.TestMethod.invoke(TestMethod.java:68) at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.runTestMethod(PowerMockJUnit44RunnerDelegateImpl.java:316) at org.junit.internal.runners.MethodRoadie$2.run(MethodRoadie.java:89) at org.junit.internal.runners.MethodRoadie.runBeforesThenTestThenAfters(MethodRoadie.java:97) at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.executeTest(PowerMockJUnit44RunnerDelegateImpl.java:300) at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.executeTestInSuper(PowerMockJUnit47RunnerDelegateImpl.java:131) at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.access$100(PowerMockJUnit47RunnerDelegateImpl.java:59) at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner$TestExecutorStatement.evaluate(PowerMockJUnit47RunnerDelegateImpl.java:147) at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.evaluateStatement(PowerMockJUnit47RunnerDelegateImpl.java:107) at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.executeTest(PowerMockJUnit47RunnerDelegateImpl.java:82) at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.runBeforesThenTestThenAfters(PowerMockJUnit44RunnerDelegateImpl.java:288) at org.junit.internal.runners.MethodRoadie.runTest(MethodRoadie.java:87) at org.junit.internal.runners.MethodRoadie.run(MethodRoadie.java:50) at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.invokeTestMethod(PowerMockJUnit44RunnerDelegateImpl.java:208) at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.runMethods(PowerMockJUnit44RunnerDelegateImpl.java:147) at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$1.run(PowerMockJUnit44RunnerDelegateImpl.java:121) at org.junit.internal.runners.ClassRoadie.runUnprotected(ClassRoadie.java:34) at org.junit.internal.runners.ClassRoadie.runProtected(ClassRoadie.java:44) at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.run(PowerMockJUnit44RunnerDelegateImpl.java:123) at org.powermock.modules.junit4.common.internal.impl.JUnit4TestSuiteChunkerImpl.run(JUnit4TestSuiteChunkerImpl.java:121) at org.powermock.modules.junit4.common.internal.impl.AbstractCommonPowerMockRunner.run(AbstractCommonPowerMockRunner.java:53) at org.powermock.modules.junit4.PowerMockRunner.run(PowerMockRunner.java:59) at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:283) at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:173) at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:153) at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:128) at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:203) at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:155) at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:103) ``` Solution The test was expecting the call to occur only once, when in-fact it could occur a few times. This is why it would fail sporadically. This doesn't impact the validity of the test. You can merge this pull request into a Git repository by running: $ git pull https://github.com/nickwallen/metron METRON-1048 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/metron/pull/657.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #657 commit a3b9369451ccd859a12983797088a7ed85ccc4d0 Author: Nick Allen <nick@nickallen.org> Date: 2017-07-18T16:39:06Z METRON-1048 Intermittent Test Failure for SimpleHBaseEnrichmentWriterLoggingTest
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jjmeyer0 commented on the issue:

          https://github.com/apache/metron/pull/657

          @nickwallen I noticed this is still failing, but on another test. I was wondering, if you mock the logger after each test would this fix the problem with the test? Only mocking it once way not reset the times the mock was interacted with.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jjmeyer0 commented on the issue: https://github.com/apache/metron/pull/657 @nickwallen I noticed this is still failing, but on another test. I was wondering, if you mock the logger after each test would this fix the problem with the test? Only mocking it once way not reset the times the mock was interacted with.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jjmeyer0 commented on the issue:

          https://github.com/apache/metron/pull/657

          Actually, after looking a bit closer I am not certain that is possible to do in this case.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jjmeyer0 commented on the issue: https://github.com/apache/metron/pull/657 Actually, after looking a bit closer I am not certain that is possible to do in this case.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user nickwallen commented on the issue:

          https://github.com/apache/metron/pull/657

          Yes, I had go back and correct a few other test cases that were doing the same thing. Thanks @jjmeyer0

          Show
          githubbot ASF GitHub Bot added a comment - Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/657 Yes, I had go back and correct a few other test cases that were doing the same thing. Thanks @jjmeyer0
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mmiklavc commented on the issue:

          https://github.com/apache/metron/pull/657

          One problem I see with this test is that it sets up the mock in a BeforeClass annotated init. That's generally frowned upon for mock tests that use verify() because expectations for the number of invocations is part of the api. A better alternative is to pull that mock out into an @Before. I rather like the idiom I've taken here best:

          ```
          public class PcapCliTest {

          @Mock
          private PcapJob jobRunner;
          @Mock
          private ResultsWriter resultsWriter;
          @Mock
          private Clock clock;

          @Before
          public void setup()

          { MockitoAnnotations.initMocks(this); }

          ```

          It keeps the setup lean and requires less boilerplate code (well, when you have multiple mocks, at least). Also, this approach will re-instantiate a new mock for every test, which I think is what we want in this case. It keeps the intent and expecations clear, to wit 1 invocation, not 1..n.

          Show
          githubbot ASF GitHub Bot added a comment - Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/657 One problem I see with this test is that it sets up the mock in a BeforeClass annotated init. That's generally frowned upon for mock tests that use verify() because expectations for the number of invocations is part of the api. A better alternative is to pull that mock out into an @Before. I rather like the idiom I've taken here best: ``` public class PcapCliTest { @Mock private PcapJob jobRunner; @Mock private ResultsWriter resultsWriter; @Mock private Clock clock; @Before public void setup() { MockitoAnnotations.initMocks(this); } ``` It keeps the setup lean and requires less boilerplate code (well, when you have multiple mocks, at least). Also, this approach will re-instantiate a new mock for every test, which I think is what we want in this case. It keeps the intent and expecations clear, to wit 1 invocation, not 1..n.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jjmeyer0 commented on the issue:

          https://github.com/apache/metron/pull/657

          @nickwallen out of curiosity, what do you think about just deleting these tests? I think the logging they are testing is great, but I don't think these are really needed.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jjmeyer0 commented on the issue: https://github.com/apache/metron/pull/657 @nickwallen out of curiosity, what do you think about just deleting these tests? I think the logging they are testing is great, but I don't think these are really needed.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mmiklavc commented on the issue:

          https://github.com/apache/metron/pull/657

          Found this useful for duplicating the problem locally.
          ```

          1. build everything depended on by metron-parsers
            cd metron
            mvn clean install -pl metron-platform/metron-parsers -am
          1. change to the parsers project
            cd metron-platform/metron-parsers
          1. keep looping on this specific test until it fails. Using macos "say" command for added fanfare
            for (( i = 1; ; i++ )); do echo "Attempt $i"; mvn test -o -Dtest=SimpleHBaseEnrichmentWriterLoggingTest; exitcode=$?; if [ $exitcode -ne 0 ]; then say test failed on attempt $i; break; fi; done
            ```
          Show
          githubbot ASF GitHub Bot added a comment - Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/657 Found this useful for duplicating the problem locally. ``` build everything depended on by metron-parsers cd metron mvn clean install -pl metron-platform/metron-parsers -am change to the parsers project cd metron-platform/metron-parsers keep looping on this specific test until it fails. Using macos "say" command for added fanfare for (( i = 1; ; i++ )); do echo "Attempt $i"; mvn test -o -Dtest=SimpleHBaseEnrichmentWriterLoggingTest; exitcode=$?; if [ $exitcode -ne 0 ]; then say test failed on attempt $i; break; fi; done ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mmiklavc commented on the issue:

          https://github.com/apache/metron/pull/657

          Another issue with this test that complicates things a bit more is that these are static calls which require more than vanilla Mockito. @jjmeyer0 might be right here. Do we really need these tests?

          Show
          githubbot ASF GitHub Bot added a comment - Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/657 Another issue with this test that complicates things a bit more is that these are static calls which require more than vanilla Mockito. @jjmeyer0 might be right here. Do we really need these tests?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mmiklavc commented on the issue:

          https://github.com/apache/metron/pull/657

          @nickwallen ok, here's the 1-line fix (2 with the static import). Ran this 30+ times in a loop with no failures.
          ```
          ...
          import static org.mockito.Mockito.reset;
          ...
          @Before
          public void setUp()

          { reset(logger); ... }

          ```

          This resets the mock counts prior to each test, thereby eliminating the problem. I'm not clear why this isn't consistently failing to begin with, but there it is.

          Show
          githubbot ASF GitHub Bot added a comment - Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/657 @nickwallen ok, here's the 1-line fix (2 with the static import). Ran this 30+ times in a loop with no failures. ``` ... import static org.mockito.Mockito.reset; ... @Before public void setUp() { reset(logger); ... } ``` This resets the mock counts prior to each test, thereby eliminating the problem. I'm not clear why this isn't consistently failing to begin with, but there it is.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user nickwallen commented on the issue:

          https://github.com/apache/metron/pull/657

          @mmiklavc I dunno, Mike. That is not working for me. I removed all the changes I made and just added the `reset` to `setUp()` and the `shouldDebugWriteOperation` fails consistently.

          Feel free to open a separate PR with your fix. Maybe I did something wrong.
          ```
          org.mockito.exceptions.verification.TooManyActualInvocations:
          logger.debug(
          <Capturing argument>,
          <Capturing argument>,
          <Capturing argument>,
          <Capturing argument>
          );
          Wanted 1 time:
          -> at org.apache.metron.writers.SimpleHBaseEnrichmentWriterLoggingTest.shouldDebugWriteOperation(SimpleHBaseEnrichmentWriterLoggingTest.java:139)
          But was 3 times. Undesired invocation:
          -> at org.apache.metron.enrichment.writer.SimpleHbaseEnrichmentWriter.getTransformer(SimpleHbaseEnrichmentWriter.java:223)
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/657 @mmiklavc I dunno, Mike. That is not working for me. I removed all the changes I made and just added the `reset` to `setUp()` and the `shouldDebugWriteOperation` fails consistently. Feel free to open a separate PR with your fix. Maybe I did something wrong. ``` org.mockito.exceptions.verification.TooManyActualInvocations: logger.debug( <Capturing argument>, <Capturing argument>, <Capturing argument>, <Capturing argument> ); Wanted 1 time: -> at org.apache.metron.writers.SimpleHBaseEnrichmentWriterLoggingTest.shouldDebugWriteOperation(SimpleHBaseEnrichmentWriterLoggingTest.java:139) But was 3 times. Undesired invocation: -> at org.apache.metron.enrichment.writer.SimpleHbaseEnrichmentWriter.getTransformer(SimpleHbaseEnrichmentWriter.java:223) ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user nickwallen commented on the issue:

          https://github.com/apache/metron/pull/657

          Honestly, I think we should just remove this test. It seems unnecessary to test when specific log statements are executed.

          Show
          githubbot ASF GitHub Bot added a comment - Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/657 Honestly, I think we should just remove this test. It seems unnecessary to test when specific log statements are executed.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user merrimanr commented on the issue:

          https://github.com/apache/metron/pull/657

          Why not just move line 60 (`logger = mock(Logger.class);`) to the setup method? Since you're testing interactions with this mock it would be better to have a new one for every test.

          Show
          githubbot ASF GitHub Bot added a comment - Github user merrimanr commented on the issue: https://github.com/apache/metron/pull/657 Why not just move line 60 (`logger = mock(Logger.class);`) to the setup method? Since you're testing interactions with this mock it would be better to have a new one for every test.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user merrimanr commented on the issue:

          https://github.com/apache/metron/pull/657

          Nevermind, LoggerFactory is static. For what it's worth, adding `org.mockito.Mockito.reset(logger);` to setUp() worked for me.

          Show
          githubbot ASF GitHub Bot added a comment - Github user merrimanr commented on the issue: https://github.com/apache/metron/pull/657 Nevermind, LoggerFactory is static. For what it's worth, adding `org.mockito.Mockito.reset(logger);` to setUp() worked for me.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user nickwallen opened a pull request:

          https://github.com/apache/metron/pull/658

          METRON-1048 Removed SimpleHBaseEnrichmentWriterLoggingTest

          Another alternative solution for METRON-1048. This simply removes the test itself. This set of tests simply test whether specific log statements are executed. I find this unnecessary.

          This also relates to #657 .

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/nickwallen/metron METRON-1048-2

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/metron/pull/658.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #658


          commit 1edc0def1e598d5cff1daca33a35422a8d197522
          Author: Nick Allen <nick@nickallen.org>
          Date: 2017-07-19T14:42:41Z

          METRON-1048 Removed SimpleHBaseEnrichmentWriterLoggingTest


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user nickwallen opened a pull request: https://github.com/apache/metron/pull/658 METRON-1048 Removed SimpleHBaseEnrichmentWriterLoggingTest Another alternative solution for METRON-1048 . This simply removes the test itself. This set of tests simply test whether specific log statements are executed. I find this unnecessary. This also relates to #657 . You can merge this pull request into a Git repository by running: $ git pull https://github.com/nickwallen/metron METRON-1048 -2 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/metron/pull/658.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #658 commit 1edc0def1e598d5cff1daca33a35422a8d197522 Author: Nick Allen <nick@nickallen.org> Date: 2017-07-19T14:42:41Z METRON-1048 Removed SimpleHBaseEnrichmentWriterLoggingTest
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user merrimanr commented on the issue:

          https://github.com/apache/metron/pull/658

          +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user merrimanr commented on the issue: https://github.com/apache/metron/pull/658 +1
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user nickwallen commented on the issue:

          https://github.com/apache/metron/pull/657

          I opened #658 to see if we have support for simply removing this set of tests. I find them unnecessary. Please review and vote on #658 whether you agree or disagree. Thanks

          Show
          githubbot ASF GitHub Bot added a comment - Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/657 I opened #658 to see if we have support for simply removing this set of tests. I find them unnecessary. Please review and vote on #658 whether you agree or disagree. Thanks
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user nickwallen commented on the issue:

          https://github.com/apache/metron/pull/658

          @mmiklavc What do you think? Just want to make sure we have consensus from everyone that chimed in on #657.

          Show
          githubbot ASF GitHub Bot added a comment - Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/658 @mmiklavc What do you think? Just want to make sure we have consensus from everyone that chimed in on #657.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user mmiklavc commented on the issue:

          https://github.com/apache/metron/pull/658

          I'm fine with that, +1. I think we have an idea what the problem was so we'll know how to fix it if we ever run into it again.

          Show
          githubbot ASF GitHub Bot added a comment - Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/658 I'm fine with that, +1. I think we have an idea what the problem was so we'll know how to fix it if we ever run into it again.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user nickwallen commented on the issue:

          https://github.com/apache/metron/pull/657

          I am closing this PR in favor of #658. This will not be merged.

          Show
          githubbot ASF GitHub Bot added a comment - Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/657 I am closing this PR in favor of #658. This will not be merged.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user nickwallen closed the pull request at:

          https://github.com/apache/metron/pull/657

          Show
          githubbot ASF GitHub Bot added a comment - Github user nickwallen closed the pull request at: https://github.com/apache/metron/pull/657
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/metron/pull/658

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/metron/pull/658

            People

            • Assignee:
              nickwallen Nick Allen
              Reporter:
              nickwallen Nick Allen
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development