SERVER-44832 Modify HookTestArchival to reset fixtures

5 files changed, 142 insertions, 78 deletions

diff --git a/buildscripts/resmokelib/testing/executor.py b/buildscripts/resmokelib/testing/executor.py
index 6821f87e674..a707e9c8b09 100644
--- a/buildscripts/resmokelib/testing/executor.py
+++ b/buildscripts/resmokelib/testing/executor.py
@@ -227,7 +227,7 @@ class TestSuiteExecutor(object):  # pylint: disable=too-many-instance-attributes
         """
         success = True
         for job in self._jobs:
-            if not job.teardown_fixture():
+            if not job.manager.teardown_fixture(self.logger):
                 self.logger.warning("Teardown of %s of job %s was not successful", job.fixture,
                                     job.job_num)
                 success = False
diff --git a/buildscripts/resmokelib/testing/hook_test_archival.py b/buildscripts/resmokelib/testing/hook_test_archival.py
index dd3ec6fbf9b..5d979a52836 100644
--- a/buildscripts/resmokelib/testing/hook_test_archival.py
+++ b/buildscripts/resmokelib/testing/hook_test_archival.py
@@ -5,6 +5,7 @@ import threading
 
 from .. import config
 from .. import utils
+from .. import errors
 from ..utils import globstar
 
 
@@ -41,21 +42,34 @@ class HookTestArchival(object):
         self._lock = threading.Lock()
 
     def _should_archive(self, success):
-        """Return True if failed test or 'on_success' is True."""
-        return not success or self.on_success
-
-    def _archive_hook(self, logger, hook, test, success):
-        """Provide helper to archive hooks."""
-        hook_match = hook.REGISTERED_NAME in self.hooks
-        if not hook_match or not self._should_archive(success):
+        """Determine whether archiving should be done."""
+        return config.ARCHIVE_FILE and self.archive_instance \
+            and (not success or self.on_success)
+
+    def _archive_hook(self, logger, result, manager):
+        """
+        Provide helper to archive hooks.
+
+        :param logger: Where the logging output should be placed.
+        :param result: A TestResult named tuple containing the test, hook, and outcome.
+        :param manager: FixtureTestCaseManager object for the calling Job.
+        """
+        if not result.hook.REGISTERED_NAME in self.hooks:
             return
 
-        test_name = "{}:{}".format(test.short_name(), hook.REGISTERED_NAME)
-        self._archive_hook_or_test(logger, test_name, test)
+        test_name = "{}:{}".format(result.test.short_name(), result.hook.REGISTERED_NAME)
+        self._archive_hook_or_test(logger, test_name, result.test, manager)
+
+    def _archive_test(self, logger, result, manager):
+        """
+        Provide helper to archive tests.
+
+        :param logger: Where the logging output should be placed.
+        :param result: A TestResult named tuple containing the test, hook, and outcome.
+        :param manager: FixtureTestCaseManager object for the calling Job.
 
-    def _archive_test(self, logger, test, success):
-        """Provide helper to archive tests."""
-        test_name = test.test_name
+        """
+        test_name = result.test.test_name
 
         if self.archive_all:
             test_match = True
@@ -67,23 +81,29 @@ class HookTestArchival(object):
                     test_match = True
                     break
 
-        if not test_match or not self._should_archive(success):
-            return
+        if test_match:
+            self._archive_hook_or_test(logger, test_name, result.test, manager)
 
-        self._archive_hook_or_test(logger, test_name, test)
+    def archive(self, logger, result, manager):
+        """
+        Archive data files for hooks or tests.
 
-    def archive(self, logger, test, success, hook=None):
-        """Archive data files for hooks or tests."""
-        if not config.ARCHIVE_FILE or not self.archive_instance:
+        :param logger: Where the logging output should be placed.
+        :param result: A TestResult named tuple containing the test, hook, and outcome.
+        :param manager: FixtureTestCaseManager object for the calling Job.
+        """
+        if not self._should_archive(result.success):
             return
-        if hook:
-            self._archive_hook(logger, hook, test, success)
+        if result.hook:
+            self._archive_hook(logger, result, manager)
         else:
-            self._archive_test(logger, test, success)
+            self._archive_test(logger, result, manager)
 
-    def _archive_hook_or_test(self, logger, test_name, test):
+    def _archive_hook_or_test(self, logger, test_name, test, manager):
         """Trigger archive of data files for a test or hook."""
 
+        # We can still attempt archiving even if the teardown fails.
+        teardown_success = manager.teardown_fixture(logger, kill=True)
         with self._lock:
             # Test repeat number is how many times the particular test has been archived.
             if test_name not in self._tests_repeat:
@@ -111,3 +131,10 @@ class HookTestArchival(object):
             logger.warning("Archive failed for %s: %s", test_name, message)
         else:
             logger.info("Archive succeeded for %s: %s", test_name, message)
+
+        setup_success = manager.setup_fixture(logger)
+        if not teardown_success:
+            raise errors.StopExecution(
+                "Error while killing test fixtures; data files may be invalid.")
+        if not setup_success:
+            raise errors.StopExecution("Error while restarting test fixtures after archiving.")
diff --git a/buildscripts/resmokelib/testing/job.py b/buildscripts/resmokelib/testing/job.py
index 312318ab5ff..1d7d76ddfdc 100644
--- a/buildscripts/resmokelib/testing/job.py
+++ b/buildscripts/resmokelib/testing/job.py
@@ -2,6 +2,7 @@
 
 import sys
 import time
+from collections import namedtuple
 
 from . import queue_element
 from . import testcases
@@ -20,14 +21,13 @@ class Job(object):  # pylint: disable=too-many-instance-attributes
             test_queue_logger):
         """Initialize the job with the specified fixture and hooks."""
 
-        self.job_num = job_num
         self.logger = logger
         self.fixture = fixture
         self.hooks = hooks
         self.report = report
         self.archival = archival
         self.suite_options = suite_options
-        self.test_queue_logger = test_queue_logger
+        self.manager = FixtureTestCaseManager(test_queue_logger, self.fixture, job_num, self.report)
 
         # Don't check fixture.is_running() when using the ContinuousStepdown hook, which kills
         # and restarts the primary. Even if the fixture is still running as expected, there is a
@@ -36,34 +36,6 @@ class Job(object):  # pylint: disable=too-many-instance-attributes
         self._check_if_fixture_running = not any(
             isinstance(hook, stepdown.ContinuousStepdown) for hook in self.hooks)
 
-    def setup_fixture(self):
-        """Run a test that sets up the job's fixture and waits for it to be ready.
-
-        Return True if the setup was successful, False otherwise.
-        """
-        test_case = _fixture.FixtureSetupTestCase(self.test_queue_logger, self.fixture,
-                                                  "job{}".format(self.job_num))
-        test_case(self.report)
-        if self.report.find_test_info(test_case).status != "pass":
-            self.logger.error("The setup of %s failed.", self.fixture)
-            return False
-
-        return True
-
-    def teardown_fixture(self):
-        """Run a test that tears down the job's fixture.
-
-        Return True if the teardown was successful, False otherwise.
-        """
-        test_case = _fixture.FixtureTeardownTestCase(self.test_queue_logger, self.fixture,
-                                                     "job{}".format(self.job_num))
-        test_case(self.report)
-        if self.report.find_test_info(test_case).status != "pass":
-            self.logger.error("The teardown of %s failed.", self.fixture)
-            return False
-
-        return True
-
     @staticmethod
     def _interrupt_all_jobs(queue, interrupt_flag):
         # Set the interrupt flag so that other jobs do not start running more tests.
@@ -84,7 +56,7 @@ class Job(object):  # pylint: disable=too-many-instance-attributes
         setup_succeeded = True
         if setup_flag is not None:
             try:
-                setup_succeeded = self.setup_fixture()
+                setup_succeeded = self.manager.setup_fixture(self.logger)
             except errors.StopExecution as err:
                 # Something went wrong when setting up the fixture. Perhaps we couldn't get a
                 # test_id from logkeeper for where to put the log output. We don't attempt to run
@@ -116,7 +88,7 @@ class Job(object):  # pylint: disable=too-many-instance-attributes
 
         if teardown_flag is not None:
             try:
-                teardown_succeeded = self.teardown_fixture()
+                teardown_succeeded = self.manager.teardown_fixture(self.logger)
             except errors.StopExecution as err:
                 # Something went wrong when tearing down the fixture. Perhaps we couldn't get a
                 # test_id from logkeeper for where to put the log output. We indicate back to the
@@ -216,7 +188,8 @@ class Job(object):  # pylint: disable=too-many-instance-attributes
         finally:
             success = self.report.find_test_info(test).status == "pass"
             if self.archival:
-                self.archival.archive(self.logger, test, success)
+                result = TestResult(test=test, hook=None, success=success)
+                self.archival.archive(self.logger, result, self.manager)
 
         self._run_hooks_after_tests(test)
 
@@ -228,7 +201,8 @@ class Job(object):  # pylint: disable=too-many-instance-attributes
             success = True
         finally:
             if self.archival:
-                self.archival.archive(self.logger, test, success, hook=hook)
+                result = TestResult(test=test, hook=hook, success=success)
+                self.archival.archive(self.logger, result, self.manager)
 
     def _run_hooks_before_tests(self, test):
         """Run the before_test method on each of the hooks.
@@ -320,3 +294,62 @@ class Job(object):  # pylint: disable=too-many-instance-attributes
             # Multiple threads may be draining the queue simultaneously, so just ignore the
             # exception from the race between queue.empty() being false and failing to get an item.
             pass
+
+
+TestResult = namedtuple('TestResult', ['test', 'hook', 'success'])
+
+
+class FixtureTestCaseManager:
+    """Class that holds information needed to create new fixture setup/teardown test cases for a single job."""
+
+    def __init__(self, test_queue_logger, fixture, job_num, report):
+        """
+        Initialize the test case manager.
+
+        :param test_queue_logger: The logger associated with this job's test queue.
+        :param fixture: The fixture associated with this job.
+        :param job_num: This job's unique identifier.
+        :param report: Report object collecting test results.
+        """
+        self.test_queue_logger = test_queue_logger
+        self.fixture = fixture
+        self.job_num = job_num
+        self.report = report
+        self.times_set_up = 0  # Setups and kills may run multiple times.
+
+    def setup_fixture(self, logger):
+        """
+        Run a test that sets up the job's fixture and waits for it to be ready.
+
+        Return True if the setup was successful, False otherwise.
+        """
+        test_case = _fixture.FixtureSetupTestCase(self.test_queue_logger, self.fixture,
+                                                  "job{}".format(self.job_num), self.times_set_up)
+        test_case(self.report)
+        if self.report.find_test_info(test_case).status != "pass":
+            logger.error("The setup of %s failed.", self.fixture)
+            return False
+
+        return True
+
+    def teardown_fixture(self, logger, kill=False):
+        """
+        Run a test that tears down the job's fixture.
+
+        Return True if the teardown was successful, False otherwise.
+        """
+        if kill:
+            test_case = _fixture.FixtureKillTestCase(self.test_queue_logger, self.fixture,
+                                                     "job{}".format(self.job_num),
+                                                     self.times_set_up)
+            self.times_set_up += 1
+        else:
+            test_case = _fixture.FixtureTeardownTestCase(self.test_queue_logger, self.fixture,
+                                                         "job{}".format(self.job_num))
+
+        test_case(self.report)
+        if self.report.find_test_info(test_case).status != "pass":
+            logger.error("The teardown of %s failed.", self.fixture)
+            return False
+
+        return True
diff --git a/buildscripts/resmokelib/testing/testcases/fixture.py b/buildscripts/resmokelib/testing/testcases/fixture.py
index fb074fd050b..346db4f3be3 100644
--- a/buildscripts/resmokelib/testing/testcases/fixture.py
+++ b/buildscripts/resmokelib/testing/testcases/fixture.py
@@ -22,9 +22,11 @@ class FixtureSetupTestCase(FixtureTestCase):
     REGISTERED_NAME = registry.LEAVE_UNREGISTERED
     PHASE = "setup"
 
-    def __init__(self, logger, fixture, job_name):
+    def __init__(self, logger, fixture, job_name, times_set_up):
         """Initialize the FixtureSetupTestCase."""
-        FixtureTestCase.__init__(self, logger, job_name, self.PHASE)
+        specific_phase = "{phase}_{times_set_up}".format(phase=self.PHASE,
+                                                         times_set_up=times_set_up)
+        FixtureTestCase.__init__(self, logger, job_name, specific_phase)
         self.fixture = fixture
 
     def run_test(self):
@@ -78,9 +80,11 @@ class FixtureKillTestCase(FixtureTestCase):
     REGISTERED_NAME = registry.LEAVE_UNREGISTERED
     PHASE = "kill"
 
-    def __init__(self, logger, fixture, job_name):
+    def __init__(self, logger, fixture, job_name, times_set_up):
         """Initialize the FixtureKillTestCase."""
-        FixtureTestCase.__init__(self, logger, job_name, self.PHASE)
+        specific_phase = "{phase}_{times_set_up}".format(phase=self.PHASE,
+                                                         times_set_up=times_set_up)
+        FixtureTestCase.__init__(self, logger, job_name, specific_phase)
         self.fixture = fixture
 
     def run_test(self):
diff --git a/buildscripts/tests/resmokelib/testing/test_job.py b/buildscripts/tests/resmokelib/testing/test_job.py
index 36ad005c997..3bd2835b8d9 100644
--- a/buildscripts/tests/resmokelib/testing/test_job.py
+++ b/buildscripts/tests/resmokelib/testing/test_job.py
@@ -221,8 +221,8 @@ class TestFixtureSetupAndTeardown(unittest.TestCase):
         # Initialize the Job instance such that its setup_fixture() and teardown_fixture() methods
         # always indicate success. The settings for these mocked method will be changed in the
         # individual test cases below.
-        self.__job_object.setup_fixture = mock.Mock(return_value=True)
-        self.__job_object.teardown_fixture = mock.Mock(return_value=True)
+        self.__job_object.manager.setup_fixture = mock.Mock(return_value=True)
+        self.__job_object.manager.teardown_fixture = mock.Mock(return_value=True)
 
     def __assert_when_run_tests(self, setup_succeeded=True, teardown_succeeded=True):
         queue = _queue.Queue()
@@ -237,37 +237,37 @@ class TestFixtureSetupAndTeardown(unittest.TestCase):
         self.assertEqual(teardown_succeeded, not teardown_flag.is_set())
 
         # teardown_fixture() should be called even if setup_fixture() raises an exception.
-        self.__job_object.setup_fixture.assert_called()
-        self.__job_object.teardown_fixture.assert_called()
+        self.__job_object.manager.setup_fixture.assert_called()
+        self.__job_object.manager.teardown_fixture.assert_called()
 
     def test_setup_and_teardown_both_succeed(self):
         self.__assert_when_run_tests()
 
     def test_setup_returns_failure(self):
-        self.__job_object.setup_fixture.return_value = False
+        self.__job_object.manager.setup_fixture.return_value = False
         self.__assert_when_run_tests(setup_succeeded=False)
 
     def test_setup_raises_logging_config_exception(self):
-        self.__job_object.setup_fixture.side_effect = errors.LoggerRuntimeConfigError(
+        self.__job_object.manager.setup_fixture.side_effect = errors.LoggerRuntimeConfigError(
             "Logging configuration error intentionally raised in unit test")
         self.__assert_when_run_tests(setup_succeeded=False)
 
     def test_setup_raises_unexpected_exception(self):
-        self.__job_object.setup_fixture.side_effect = Exception(
+        self.__job_object.manager.setup_fixture.side_effect = Exception(
             "Generic error intentionally raised in unit test")
         self.__assert_when_run_tests(setup_succeeded=False)
 
     def test_teardown_returns_failure(self):
-        self.__job_object.teardown_fixture.return_value = False
+        self.__job_object.manager.teardown_fixture.return_value = False
         self.__assert_when_run_tests(teardown_succeeded=False)
 
     def test_teardown_raises_logging_config_exception(self):
-        self.__job_object.teardown_fixture.side_effect = errors.LoggerRuntimeConfigError(
+        self.__job_object.manager.teardown_fixture.side_effect = errors.LoggerRuntimeConfigError(
             "Logging configuration error intentionally raised in unit test")
         self.__assert_when_run_tests(teardown_succeeded=False)
 
     def test_teardown_raises_unexpected_exception(self):
-        self.__job_object.teardown_fixture.side_effect = Exception(
+        self.__job_object.manager.teardown_fixture.side_effect = Exception(
             "Generic error intentionally raised in unit test")
         self.__assert_when_run_tests(teardown_succeeded=False)
 
@@ -276,22 +276,22 @@ class TestNoOpFixtureSetupAndTeardown(unittest.TestCase):
     """Test cases for NoOpFixture handling in setup_fixture() and teardown_fixture()."""
 
     def setUp(self):
-        logger = logging.getLogger("job_unittest")
-        self.__noop_fixture = _fixtures.NoOpFixture(logger=logger, job_num=0)
+        self.logger = logging.getLogger("job_unittest")
+        self.__noop_fixture = _fixtures.NoOpFixture(logger=self.logger, job_num=0)
         self.__noop_fixture.setup = mock.Mock()
         self.__noop_fixture.teardown = mock.Mock()
 
         test_report = mock.Mock()
         test_report.find_test_info().status = "pass"
 
-        self.__job_object = job.Job(job_num=0, logger=logger, fixture=self.__noop_fixture, hooks=[],
-                                    report=test_report, archival=None, suite_options=None,
-                                    test_queue_logger=logger)
+        self.__job_object = job.Job(job_num=0, logger=self.logger, fixture=self.__noop_fixture,
+                                    hooks=[], report=test_report, archival=None, suite_options=None,
+                                    test_queue_logger=self.logger)
 
     def test_setup_called_for_noop_fixture(self):
-        self.assertTrue(self.__job_object.setup_fixture())
+        self.assertTrue(self.__job_object.manager.setup_fixture(self.logger))
         self.__noop_fixture.setup.assert_called_once_with()
 
     def test_teardown_called_for_noop_fixture(self):
-        self.assertTrue(self.__job_object.teardown_fixture())
+        self.assertTrue(self.__job_object.manager.teardown_fixture(self.logger))
         self.__noop_fixture.teardown.assert_called_once_with(finished=True)