Details

    • Type: Task Task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.3, 6.0
    • Component/s: SolrCloud, Tests
    • Labels:
      None

      Description

      I see many tests which blindly have distribSetUp and distribTearDown methods setting a variety of options and system properties that aren't required anymore.

      This is because some base test classes have been refactored such that these options are redundant. In other cases, people have copied the structure of tests blindly instead of understanding what each parameter does.

      Let's try to remove the unnecessary config params from such tests.

      1. SOLR-7599.patch
        31 kB
        Shalin Shekhar Mangar

        Activity

        Hide
        Shalin Shekhar Mangar added a comment -

        Here's a list of stuff that should be looked at:

        1. Does a test really need distribSetUp and distribTearDown methods?
        2. Usefulness of useJettyDataDir
        3. solr.xml.persist
        4. The need to change the value for DirectUpdateHandler2.commitOnClose. Only a few tests really need it.
        5. Setting system properties such as System.setProperty("numShards", Integer.toString(sliceCount));
        6. checkCreatedVsState – looks like it is always false across all tests.
        Show
        Shalin Shekhar Mangar added a comment - Here's a list of stuff that should be looked at: Does a test really need distribSetUp and distribTearDown methods? Usefulness of useJettyDataDir solr.xml.persist The need to change the value for DirectUpdateHandler2.commitOnClose. Only a few tests really need it. Setting system properties such as System.setProperty("numShards", Integer.toString(sliceCount)); checkCreatedVsState – looks like it is always false across all tests.
        Hide
        Shalin Shekhar Mangar added a comment -

        Initial cut which removes all redundant distribSetup and distribTearDown method calls.

        Noble Paul - Regarding ExternalCollectionsTest – is it useful anymore given that "external" collections are switched on/off randomly across all tests? Perhaps we should move the .system collection testing to its own test and delete this one.

        Show
        Shalin Shekhar Mangar added a comment - Initial cut which removes all redundant distribSetup and distribTearDown method calls. Noble Paul - Regarding ExternalCollectionsTest – is it useful anymore given that "external" collections are switched on/off randomly across all tests? Perhaps we should move the .system collection testing to its own test and delete this one.
        Hide
        Shalin Shekhar Mangar added a comment -

        There's an oddity in CloudSolrClientTest which has the following:

          @BeforeClass
          public static void beforeSuperClass() {
              AbstractZkTestCase.SOLRHOME = new File(SOLR_HOME());
          }
        

        Removing the above fails the test. There is no apparent link between this test and AbstractZkTestCase.SOLRHOME until I found that this property is used to setup ZK by the parent base test and that the auto-detected value of AbstractZkTestCase.SOLRHOME applies only to core tests and not solrj tests. I have put in this explanation as a comment in that method but we should try to un-entangle such things in another issue.

        Show
        Shalin Shekhar Mangar added a comment - There's an oddity in CloudSolrClientTest which has the following: @BeforeClass public static void beforeSuperClass() { AbstractZkTestCase.SOLRHOME = new File(SOLR_HOME()); } Removing the above fails the test. There is no apparent link between this test and AbstractZkTestCase.SOLRHOME until I found that this property is used to setup ZK by the parent base test and that the auto-detected value of AbstractZkTestCase.SOLRHOME applies only to core tests and not solrj tests. I have put in this explanation as a comment in that method but we should try to un-entangle such things in another issue.
        Hide
        Noble Paul added a comment -

        Yes, it is required. It ensures that the state is indeed saved in the state.json .

        Show
        Noble Paul added a comment - Yes, it is required. It ensures that the state is indeed saved in the state.json .
        Hide
        ASF subversion and git services added a comment -

        Commit 1682168 from shalin@apache.org in branch 'dev/trunk'
        [ https://svn.apache.org/r1682168 ]

        SOLR-7599: Remove cruft from SolrCloud tests

        Show
        ASF subversion and git services added a comment - Commit 1682168 from shalin@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1682168 ] SOLR-7599 : Remove cruft from SolrCloud tests
        Hide
        Shalin Shekhar Mangar added a comment -

        Yes, it is required. It ensures that the state is indeed saved in the state.json .

        Okay, I will rename ExternalCollectionsTest to CollectionStateFormat2Test. I will also move the .system collection test from this test file to its own so that it is easier to find. Better not to mix unrelated tests together in test class.

        I also noticed that DeleteLastCustomShardedReplicaTest is ignored on trunk but not on branch_5x. Since I haven't seen this test failing on branch_5x, I will re-enable this test on trunk and investigate if it fails again.

        Show
        Shalin Shekhar Mangar added a comment - Yes, it is required. It ensures that the state is indeed saved in the state.json . Okay, I will rename ExternalCollectionsTest to CollectionStateFormat2Test. I will also move the .system collection test from this test file to its own so that it is easier to find. Better not to mix unrelated tests together in test class. I also noticed that DeleteLastCustomShardedReplicaTest is ignored on trunk but not on branch_5x. Since I haven't seen this test failing on branch_5x, I will re-enable this test on trunk and investigate if it fails again.
        Hide
        ASF subversion and git services added a comment -

        Commit 1682173 from shalin@apache.org in branch 'dev/trunk'
        [ https://svn.apache.org/r1682173 ]

        SOLR-6593: SOLR-7599: Re-enable DeleteLastCustomShardedReplicaTest on trunk

        Show
        ASF subversion and git services added a comment - Commit 1682173 from shalin@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1682173 ] SOLR-6593 : SOLR-7599 : Re-enable DeleteLastCustomShardedReplicaTest on trunk
        Hide
        ASF subversion and git services added a comment -

        Commit 1682174 from shalin@apache.org in branch 'dev/trunk'
        [ https://svn.apache.org/r1682174 ]

        SOLR-7599: Rename ExternalCollectionsTest to CollectionStateFormat2Test

        Show
        ASF subversion and git services added a comment - Commit 1682174 from shalin@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1682174 ] SOLR-7599 : Rename ExternalCollectionsTest to CollectionStateFormat2Test
        Hide
        ASF subversion and git services added a comment -

        Commit 1682181 from shalin@apache.org in branch 'dev/trunk'
        [ https://svn.apache.org/r1682181 ]

        SOLR-7599: Removed some distribTearDown methods that were left over

        Show
        ASF subversion and git services added a comment - Commit 1682181 from shalin@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1682181 ] SOLR-7599 : Removed some distribTearDown methods that were left over
        Hide
        ASF subversion and git services added a comment -

        Commit 1682258 from shalin@apache.org in branch 'dev/trunk'
        [ https://svn.apache.org/r1682258 ]

        SOLR-7599: More clean up of duplicate methods, renamed some methods and variable names

        Show
        ASF subversion and git services added a comment - Commit 1682258 from shalin@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1682258 ] SOLR-7599 : More clean up of duplicate methods, renamed some methods and variable names
        Hide
        ASF subversion and git services added a comment -

        Commit 1682340 from shalin@apache.org in branch 'dev/trunk'
        [ https://svn.apache.org/r1682340 ]

        SOLR-7599: Inline startCloudJetty method into ShardRoutingCustomTest

        Show
        ASF subversion and git services added a comment - Commit 1682340 from shalin@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1682340 ] SOLR-7599 : Inline startCloudJetty method into ShardRoutingCustomTest
        Hide
        David Smiley added a comment -

        Cleaning up crap like this is usually a thankless task, but I hereby thank you for it!

        Show
        David Smiley added a comment - Cleaning up crap like this is usually a thankless task, but I hereby thank you for it!
        Hide
        ASF subversion and git services added a comment -

        Commit 1683417 from shalin@apache.org in branch 'dev/trunk'
        [ https://svn.apache.org/r1683417 ]

        SOLR-7599: Remove invokeCollectionAPI method from base test class and use solrj classes throughout

        Show
        ASF subversion and git services added a comment - Commit 1683417 from shalin@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1683417 ] SOLR-7599 : Remove invokeCollectionAPI method from base test class and use solrj classes throughout
        Hide
        ASF subversion and git services added a comment -

        Commit 1683987 from shalin@apache.org in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1683987 ]

        SOLR-7599: Remove cruft from SolrCloud tests

        Show
        ASF subversion and git services added a comment - Commit 1683987 from shalin@apache.org in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1683987 ] SOLR-7599 : Remove cruft from SolrCloud tests
        Hide
        Shalin Shekhar Mangar added a comment -

        Bulk close for 5.3.0 release

        Show
        Shalin Shekhar Mangar added a comment - Bulk close for 5.3.0 release

          People

          • Assignee:
            Shalin Shekhar Mangar
            Reporter:
            Shalin Shekhar Mangar
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development