Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-11053

Improve SoftAutoCommitTest to the point that we can delete AutoCommitTest and HardAutoCommitTest

Details

    • Improvement
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 7.2, 8.0
    • None
    • None

    Description

      This jira serves as a parent with the objective of being able to delete AutoCommitTest and HardAutoCommitTest – w/o any loss in what core functionality is being tested – by adding equivilent test logic to SoftAutoCommitTest using the (superior) strategies currently found in SoftAutoCommitTest.

      Sub-Tasks will be used to track the discrete changes to be made, so we can phase them in gradually.

      Attachments

        1. SOLR-11053.patch
          20 kB
          Chris M. Hostetter

        Issue Links

          Activity

            When Miller first wrote SoftAutoCommitTest back in 2011, it was with the expressly started purpose to "Test auto commit functionality in a way that doesn't suck."

            As the javadocs put it...

            /**
             * Test auto commit functionality in a way that doesn't suck.
             * <p>
             * AutoCommitTest is an abomination that is way to brittle in how it 
             * tries to check that commits happened, and when they happened.
             * The goal of this test class is to (ultimately) completely replace all 
             * of the functionality of that test class using:
             * </p>
             * <ul>
             *   <li>A more robust monitor of commit/newSearcher events that records 
             *       the times of those events in a queue that can be polled.  
             *       Multiple events in rapid succession are not lost.
             *   </li>
             *   <li>Timing checks that are forgiving of slow machines and use 
             *       knowledge of how slow A-&gt;B was to affect the expectation of 
             *       how slow B-&gt;C will be
             *   </li>
             * </ul>
             */
            

            The jenkins failure emails (especially fromthe past few months) back up the implication that the strategy used in SoftAutoCommitTest is much hardier (even on flaky machines) then the approaches taken in AutoCommitTest and HardAutoCommitTest.


            AFAICT there are esentially only 3 things these "bad" tests currently check that aren't already covered by the existing logic in SoftAutoCommitTest:

            • maxDocs
              • AutoCommitTest.testMaxDocs() currently checks the "softCommit" aspect (via newSearcher)
              • nothing currently seems to test the "hardCommit" side
            • commitWithin
              • AutoCommitTest.testCommitWithin() currently checks the "softCommit" aspect (via newSearcher)
              • HardAutoCommitTest.testCommitWithin() currently checks the "hardCommit" aspect (but also via newSearcher)

            ...I'll put my thoughts on these in individual sub-tasks

            hossman Chris M. Hostetter added a comment - When Miller first wrote SoftAutoCommitTest back in 2011, it was with the expressly started purpose to "Test auto commit functionality in a way that doesn't suck." As the javadocs put it... /** * Test auto commit functionality in a way that doesn't suck. * <p> * AutoCommitTest is an abomination that is way to brittle in how it * tries to check that commits happened, and when they happened. * The goal of this test class is to (ultimately) completely replace all * of the functionality of that test class using: * </p> * <ul> * <li>A more robust monitor of commit/newSearcher events that records * the times of those events in a queue that can be polled. * Multiple events in rapid succession are not lost. * </li> * <li>Timing checks that are forgiving of slow machines and use * knowledge of how slow A-&gt;B was to affect the expectation of * how slow B-&gt;C will be * </li> * </ul> */ The jenkins failure emails (especially fromthe past few months) back up the implication that the strategy used in SoftAutoCommitTest is much hardier (even on flaky machines) then the approaches taken in AutoCommitTest and HardAutoCommitTest. AFAICT there are esentially only 3 things these "bad" tests currently check that aren't already covered by the existing logic in SoftAutoCommitTest: maxDocs AutoCommitTest.testMaxDocs() currently checks the "softCommit" aspect (via newSearcher) nothing currently seems to test the "hardCommit" side commitWithin AutoCommitTest.testCommitWithin() currently checks the "softCommit" aspect (via newSearcher) HardAutoCommitTest.testCommitWithin() currently checks the "hardCommit" aspect (but also via newSearcher) ...I'll put my thoughts on these in individual sub-tasks

            now that the subtasks are resolved, I think we're ready to purge AutoCommitTest and HardAutoCommitTest from our code base.

            trivially straight forward patch attached ... precommit passes.

            hossman Chris M. Hostetter added a comment - now that the subtasks are resolved, I think we're ready to purge AutoCommitTest and HardAutoCommitTest from our code base. trivially straight forward patch attached ... precommit passes.

            Commit 1873871b7458009ebdb88c48428b263830752a3d in lucene-solr's branch refs/heads/branch_7x from Chris Hostetter
            [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=1873871 ]

            SOLR-11053: remove AutoCommitTest + HardAutoCommitTest now that SoftAutoCommitTest exercises all the same functionality with more robust assertions

            (cherry picked from commit 2da777cdb89c45a69081452ec4efb3e6b61108b6)

            jira-bot ASF subversion and git services added a comment - Commit 1873871b7458009ebdb88c48428b263830752a3d in lucene-solr's branch refs/heads/branch_7x from Chris Hostetter [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=1873871 ] SOLR-11053 : remove AutoCommitTest + HardAutoCommitTest now that SoftAutoCommitTest exercises all the same functionality with more robust assertions (cherry picked from commit 2da777cdb89c45a69081452ec4efb3e6b61108b6)

            Commit 2da777cdb89c45a69081452ec4efb3e6b61108b6 in lucene-solr's branch refs/heads/master from Chris Hostetter
            [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2da777c ]

            SOLR-11053: remove AutoCommitTest + HardAutoCommitTest now that SoftAutoCommitTest exercises all the same functionality with more robust assertions

            jira-bot ASF subversion and git services added a comment - Commit 2da777cdb89c45a69081452ec4efb3e6b61108b6 in lucene-solr's branch refs/heads/master from Chris Hostetter [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2da777c ] SOLR-11053 : remove AutoCommitTest + HardAutoCommitTest now that SoftAutoCommitTest exercises all the same functionality with more robust assertions

            People

              hossman Chris M. Hostetter
              hossman Chris M. Hostetter
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: