Uploaded image for project: 'Stratos'
  1. Stratos
  2. STRATOS-7

Autoscale Mediator BE - AppDomainContextsTest Error

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0.0
    • Component/s: None
    • Labels:
      None

      Description

      AppDomainContextsTest in Autoscale Mediator BE is using the singleton LB configuration instance and it's service configurations added in one test method are affecting the other test methods.

      We might need to isolate and remove service configurations added in a test method either in tearDown() or at the end of the test.

      Error:
      -------------------------------------------------------------------------------
      Test set: org.apache.stratos.mediator.autoscale.lbautoscale.AppDomainContextsTest
      -------------------------------------------------------------------------------
      Tests run: 2, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.215 sec <<< FAILURE!
      testRemoval(org.apache.stratos.mediator.autoscale.lbautoscale.AppDomainContextsTest) Time elapsed: 0.003 sec <<< ERROR!
      org.apache.synapse.SynapseException: Axis2 clustering GroupManagementAgent for domain: wso2.as3.domain, sub-domain: mgt has not been defined
      at org.apache.stratos.mediator.autoscale.lbautoscale.util.AutoscaleUtil.getAppDomainContexts(AutoscaleUtil.java:256)
      at org.apache.stratos.mediator.autoscale.lbautoscale.AppDomainContextsTest.setUp(AppDomainContextsTest.java:63)
      at junit.framework.TestCase.runBare(TestCase.java:139)
      at junit.framework.TestResult$1.protect(TestResult.java:122)
      at junit.framework.TestResult.runProtected(TestResult.java:142)
      at junit.framework.TestResult.run(TestResult.java:125)
      at junit.framework.TestCase.run(TestCase.java:129)
      at junit.framework.TestSuite.runTest(TestSuite.java:255)
      at junit.framework.TestSuite.run(TestSuite.java:250)
      at org.junit.internal.runners.JUnit38ClassRunner.run(JUnit38ClassRunner.java:84)
      at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:236)
      at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:134)
      at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:113)
      at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
      at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
      at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
      at java.lang.reflect.Method.invoke(Method.java:597)
      at org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:189)
      at org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:165)
      at org.apache.maven.surefire.booter.ProviderFactory.invokeProvider(ProviderFactory.java:85)
      at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:103)
      at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:74)

        Activity

        Hide
        chrishantha Isuru Perera added a comment -

        I applied the patch "0001-Updated-test-case-to-use-junit4-and-added-tear-down-.patch" with revision 98bfde131ec1e5b1ffbaafb65ad5487edfa236cc

        Thanks!

        Show
        chrishantha Isuru Perera added a comment - I applied the patch "0001-Updated-test-case-to-use-junit4-and-added-tear-down-.patch" with revision 98bfde131ec1e5b1ffbaafb65ad5487edfa236cc Thanks!
        Hide
        imesh Imesh Gunaratne added a comment -

        Please apply the attached patch. Thanks.

        Show
        imesh Imesh Gunaratne added a comment - Please apply the attached patch. Thanks.
        Hide
        imesh Imesh Gunaratne added a comment - - edited

        Fixed:

        • Updated test case to use junit4 and added tear down functionality
        • Now the setup and tear down are only run once.
        • Attached patch file: 0001-Updated-test-case-to-use-junit4-and-added-tear-down-.patch
        Show
        imesh Imesh Gunaratne added a comment - - edited Fixed: Updated test case to use junit4 and added tear down functionality Now the setup and tear down are only run once. Attached patch file: 0001-Updated-test-case-to-use-junit4-and-added-tear-down-.patch
        Hide
        imesh Imesh Gunaratne added a comment -

        Hi Nirmal,

        Yes I agree, as we discussed will add tearDown() method to clear service configurations.

        Thanks

        Show
        imesh Imesh Gunaratne added a comment - Hi Nirmal, Yes I agree, as we discussed will add tearDown() method to clear service configurations. Thanks
        Hide
        nirmal Nirmal Fernando added a comment -

        Hi Imesh,

        It depends on to which level of granularity we go for. IMO it is too fine grained, if we call setUp for each method in a test class. All of the tests inside this class are correlated, hence they've added in the same class. And, setUp should be called only once before this class starts to execute its tests. Further, frequent cleaning and setting up may slow down the build process drastically.

        WDYT?

        Show
        nirmal Nirmal Fernando added a comment - Hi Imesh, It depends on to which level of granularity we go for. IMO it is too fine grained, if we call setUp for each method in a test class. All of the tests inside this class are correlated, hence they've added in the same class. And, setUp should be called only once before this class starts to execute its tests. Further, frequent cleaning and setting up may slow down the build process drastically. WDYT?
        Hide
        imesh Imesh Gunaratne added a comment -

        Hi Nirmal,

        Thanks for the feedback. I do not think that using @BeforeClass would fix the main cause of the issue. If we implement another test case by accessing the LB Config instance and add more service configurations it might fail again. The reason for this is that LB Config instance being singleton.

        The better option would be to clean up configuration data once the test is done with its execution.

        Regarding the JUnit version, I think it should not matter if we clean the configuration data as described above.

        Thanks

        Show
        imesh Imesh Gunaratne added a comment - Hi Nirmal, Thanks for the feedback. I do not think that using @BeforeClass would fix the main cause of the issue. If we implement another test case by accessing the LB Config instance and add more service configurations it might fail again. The reason for this is that LB Config instance being singleton. The better option would be to clean up configuration data once the test is done with its execution. Regarding the JUnit version, I think it should not matter if we clean the configuration data as described above. Thanks
        Hide
        nirmal Nirmal Fernando added a comment -

        Please do not downgrade the junit version, but let's properly adapt to the new version (for example use of annotation in this scenario).

        Show
        nirmal Nirmal Fernando added a comment - Please do not downgrade the junit version, but let's properly adapt to the new version (for example use of annotation in this scenario).
        Hide
        chrishantha Isuru Perera added a comment -

        Hi Nirmal,

        Good catch! I upgraded the JUnit version. A test error came earlier on Java 7 (for Imesh), which was before the version upgrade. So, I didn't think that the version upgrade will affect here. I'm sorry for missing this!

        I will then revert the JUnit version change and also revert the change attached here. We might need to properly update JUnit version later, since this version change might affect other test cases too!

        Thanks!

        Show
        chrishantha Isuru Perera added a comment - Hi Nirmal, Good catch! I upgraded the JUnit version. A test error came earlier on Java 7 (for Imesh), which was before the version upgrade. So, I didn't think that the version upgrade will affect here. I'm sorry for missing this! I will then revert the JUnit version change and also revert the change attached here. We might need to properly update JUnit version later, since this version change might affect other test cases too! Thanks!
        Hide
        nirmal Nirmal Fernando added a comment -

        Hi Imesh,

        After looking at the stack trace .. I can confirm that this is a regression introduced by junit version upgrade and correct fix would be to add @BeforeClass annotation for setUp method.

        Thanks,
        Nirmal

        Show
        nirmal Nirmal Fernando added a comment - Hi Imesh, After looking at the stack trace .. I can confirm that this is a regression introduced by junit version upgrade and correct fix would be to add @BeforeClass annotation for setUp method. Thanks, Nirmal
        Hide
        imesh Imesh Gunaratne added a comment -

        Hi Nirmal,

        Yes for sure, the problem was like this:

        In this test case there are two test methods:
        1. testAddition()
        2. testRemoval()

        Here both test methods are using the same LB config instance (singleton). When we execute the test, it first execute the testAddition() and adds a service configuration. Then the addition test logic is executed successfully.

        Then the second test method testRemoval() is invoked. At this point, LB config instance contains the service configuration added by the testAddition() method. I think this is not correct.

        In Unit Testing I think it is always better if we could execute a given test logic within it's method and dispose all objects. Each test method should not depend on other test methods.

        However in this scenario since we have to use a singleton instance we may need to clean up all the config data added to it when we end the test logic.

        Thanks

        Show
        imesh Imesh Gunaratne added a comment - Hi Nirmal, Yes for sure, the problem was like this: In this test case there are two test methods: 1. testAddition() 2. testRemoval() Here both test methods are using the same LB config instance (singleton). When we execute the test, it first execute the testAddition() and adds a service configuration. Then the addition test logic is executed successfully. Then the second test method testRemoval() is invoked. At this point, LB config instance contains the service configuration added by the testAddition() method. I think this is not correct. In Unit Testing I think it is always better if we could execute a given test logic within it's method and dispose all objects. Each test method should not depend on other test methods. However in this scenario since we have to use a singleton instance we may need to clean up all the config data added to it when we end the test logic. Thanks
        Hide
        chrishantha Isuru Perera added a comment -

        Hi,

        I applied the patch since others get build errors.

        Check revision 2f793dc0ed9990696a1531dcccd3d8c21beaf00e.

        This change is not necessary since the test runs fine in WSO2 code. We need to carefully analyze what's wrong. I'll update the JIRA description with error.

        Thanks!

        Show
        chrishantha Isuru Perera added a comment - Hi, I applied the patch since others get build errors. Check revision 2f793dc0ed9990696a1531dcccd3d8c21beaf00e. This change is not necessary since the test runs fine in WSO2 code. We need to carefully analyze what's wrong. I'll update the JIRA description with error. Thanks!
        Hide
        nirmal Nirmal Fernando added a comment -

        Hi Imesh,

        Would you be able to explain, what test was failed and the stack trace. It is always better to update the description, with stack trace etc. in order to get a grasp of the issue easily and also to resolve similar issues lately.

        Show
        nirmal Nirmal Fernando added a comment - Hi Imesh, Would you be able to explain, what test was failed and the stack trace. It is always better to update the description, with stack trace etc. in order to get a grasp of the issue easily and also to resolve similar issues lately.
        Hide
        imesh Imesh Gunaratne added a comment -

        Hi Nirmal, I'm sorry I could not update the issue description after my last comment. Please refer 0001-Removed-service-configuration-at-the-end-of-test-add.patch.
        Thanks

        Show
        imesh Imesh Gunaratne added a comment - Hi Nirmal, I'm sorry I could not update the issue description after my last comment. Please refer 0001-Removed-service-configuration-at-the-end-of-test-add.patch. Thanks
        Hide
        nirmal Nirmal Fernando added a comment -

        Hi Imesh,

        I'm sorry, I don't understand the statement "AppDomainContextsTest in Autoscale Mediator BE does not execute its setUp() method. As a result testRemoval() fails.". You do not need to call setUp explicitly, it is supposed to get called by the test engine.
        Can you elaborate please?

        Nirmal

        Show
        nirmal Nirmal Fernando added a comment - Hi Imesh, I'm sorry, I don't understand the statement "AppDomainContextsTest in Autoscale Mediator BE does not execute its setUp() method. As a result testRemoval() fails.". You do not need to call setUp explicitly, it is supposed to get called by the test engine. Can you elaborate please? Nirmal
        Hide
        imesh Imesh Gunaratne added a comment -

        As I found the problem has been caused by the singleton LB configuration instance used by two test methods.
        Fixed: Removed service configuration wso2.as3.domain, mgt at the end of test addition

        Please apply the patch:
        0001-Removed-service-configuration-at-the-end-of-test-add.patch

        Thanks

        Show
        imesh Imesh Gunaratne added a comment - As I found the problem has been caused by the singleton LB configuration instance used by two test methods. Fixed: Removed service configuration wso2.as3.domain, mgt at the end of test addition Please apply the patch: 0001-Removed-service-configuration-at-the-end-of-test-add.patch Thanks
        Hide
        imesh Imesh Gunaratne added a comment -

        I will verify this Isuru. Thanks

        Show
        imesh Imesh Gunaratne added a comment - I will verify this Isuru. Thanks
        Hide
        chrishantha Isuru Perera added a comment -

        I still get this error even after applying the patch.

        Show
        chrishantha Isuru Perera added a comment - I still get this error even after applying the patch.
        Hide
        chrishantha Isuru Perera added a comment -

        Applied patch with revision 1dd40e7624eb967ba2c88bcd44de7f455590079e

        Thanks!

        Show
        chrishantha Isuru Perera added a comment - Applied patch with revision 1dd40e7624eb967ba2c88bcd44de7f455590079e Thanks!
        Hide
        imesh Imesh Gunaratne added a comment -

        Fixed and attached patch file.

        Show
        imesh Imesh Gunaratne added a comment - Fixed and attached patch file.

          People

          • Assignee:
            imesh Imesh Gunaratne
            Reporter:
            imesh Imesh Gunaratne
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development