From 8c3d4ce85ce9bc3a432500774b3ebc5d7eda1e47 Mon Sep 17 00:00:00 2001
From: Calen Pennington <cale@edx.org>
Date: Wed, 20 Jul 2016 13:37:11 -0400
Subject: [PATCH] Split actions needed for bok_choy tests into smaller tasks to
 get more granular timing information

---
 pavelib/acceptance_test.py                    |   2 +
 pavelib/bok_choy.py                           |  22 ++-
 pavelib/utils/test/bokchoy_options.py         |   3 +-
 pavelib/utils/test/bokchoy_utils.py           |  23 ++-
 pavelib/utils/test/suites/acceptance_suite.py | 110 +++++++------
 pavelib/utils/test/suites/bokchoy_suite.py    | 151 ++++++++++--------
 pavelib/utils/test/suites/suite.py            |   8 -
 7 files changed, 178 insertions(+), 141 deletions(-)

diff --git a/pavelib/acceptance_test.py b/pavelib/acceptance_test.py
index 271c1e6c892..ceca56e5ccb 100644
--- a/pavelib/acceptance_test.py
+++ b/pavelib/acceptance_test.py
@@ -4,6 +4,7 @@ Acceptance test tasks
 from paver.easy import cmdopts, needs
 from pavelib.utils.test.suites import AcceptanceTestSuite
 from pavelib.utils.passthrough_opts import PassthroughTask
+from pavelib.utils.timer import timed
 from optparse import make_option
 
 try:
@@ -29,6 +30,7 @@ __test__ = False  # do not collect
     ('extra_args=', 'e', 'deprecated, pass extra options directly in the paver commandline'),
 ])
 @PassthroughTask
+@timed
 def test_acceptance(options, passthrough_options):
     """
     Run the acceptance tests for either lms or cms
diff --git a/pavelib/bok_choy.py b/pavelib/bok_choy.py
index ef7509c8e7a..dffa9b24f09 100644
--- a/pavelib/bok_choy.py
+++ b/pavelib/bok_choy.py
@@ -71,10 +71,12 @@ def test_a11y(options, passthrough_options):
     It can also be left blank to run all tests in the suite that are tagged
     with `@attr("a11y")`.
     """
+    # Modify the options object directly, so that any subsequently called tasks
+    # that share with this task get the modified options
+    options['report_dir'] = Env.BOK_CHOY_A11Y_REPORT_DIR
+    options['coveragerc'] = Env.BOK_CHOY_A11Y_COVERAGERC
+    options['extra_args'] = options.get('extra_args', '') + ' -a "a11y" '
     opts = parse_bokchoy_opts(options, passthrough_options)
-    opts['report_dir'] = Env.BOK_CHOY_A11Y_REPORT_DIR
-    opts['coveragerc'] = Env.BOK_CHOY_A11Y_COVERAGERC
-    opts['extra_args'] = opts['extra_args'] + ' -a "a11y" '
     run_bokchoy(**opts)
 
 
@@ -86,8 +88,10 @@ def perf_report_bokchoy(options, passthrough_options):
     """
     Generates a har file for with page performance info.
     """
+    # Modify the options object directly, so that any subsequently called tasks
+    # that share with this task get the modified options
+    options['test_dir'] = 'performance'
     opts = parse_bokchoy_opts(options, passthrough_options)
-    opts['test_dir'] = 'performance'
 
     run_bokchoy(**opts)
 
@@ -114,11 +118,13 @@ def pa11ycrawler(options, passthrough_options):
     flag to get an environment running. The setup for this is the same as
     for bok-choy tests, only test course is imported as well.
     """
+    # Modify the options object directly, so that any subsequently called tasks
+    # that share with this task get the modified options
+    options['report_dir'] = Env.PA11YCRAWLER_REPORT_DIR
+    options['coveragerc'] = Env.PA11YCRAWLER_COVERAGERC
+    options['should_fetch_course'] = getattr(options, 'should_fetch_course', not options.get('fasttest'))
+    options['course_key'] = getattr(options, 'course-key', "course-v1:edX+Test101+course")
     opts = parse_bokchoy_opts(options, passthrough_options)
-    opts['report_dir'] = Env.PA11YCRAWLER_REPORT_DIR
-    opts['coveragerc'] = Env.PA11YCRAWLER_COVERAGERC
-    opts['should_fetch_course'] = getattr(options, 'should_fetch_course', not opts['fasttest'])
-    opts['course_key'] = getattr(options, 'course-key', "course-v1:edX+Test101+course")
     test_suite = Pa11yCrawler('a11y_crawler', **opts)
     test_suite.run()
 
diff --git a/pavelib/utils/test/bokchoy_options.py b/pavelib/utils/test/bokchoy_options.py
index b684c3ac48c..ffa46ac9443 100644
--- a/pavelib/utils/test/bokchoy_options.py
+++ b/pavelib/utils/test/bokchoy_options.py
@@ -64,5 +64,6 @@ def parse_bokchoy_opts(options, passthrough_options=None):
         'test_dir': getattr(options, 'test_dir', 'tests'),
         'imports_dir': getattr(options, 'imports_dir', None),
         'save_screenshots': getattr(options, 'save_screenshots', False),
-        'passthrough_options': passthrough_options
+        'passthrough_options': passthrough_options,
+        'report_dir': getattr(options, 'report_dir', Env.BOK_CHOY_REPORT_DIR),
     }
diff --git a/pavelib/utils/test/bokchoy_utils.py b/pavelib/utils/test/bokchoy_utils.py
index 7aac1ec6064..8a4117e4fb0 100644
--- a/pavelib/utils/test/bokchoy_utils.py
+++ b/pavelib/utils/test/bokchoy_utils.py
@@ -6,9 +6,11 @@ import os
 import time
 import httplib
 import subprocess
-from paver.easy import sh
+from paver.easy import sh, task, cmdopts
 from pavelib.utils.envs import Env
 from pavelib.utils.process import run_background_process
+from pavelib.utils.test.bokchoy_options import BOKCHOY_OPTS
+from pavelib.utils.timer import timed
 
 try:
     from pygments.console import colorize
@@ -18,11 +20,14 @@ except ImportError:
 __test__ = False  # do not collect
 
 
-def start_servers(default_store, coveragerc=None):
+@task
+@cmdopts(BOKCHOY_OPTS, share_with=['test_bokchoy', 'test_a11y', 'pa11ycrawler'])
+@timed
+def start_servers(options):
     """
     Start the servers we will run tests on, returns PIDs for servers.
     """
-    coveragerc = coveragerc or Env.BOK_CHOY_COVERAGERC
+    coveragerc = options.get('coveragerc', Env.BOK_CHOY_COVERAGERC)
 
     def start_server(cmd, logfile, cwd=None):
         """
@@ -38,7 +43,7 @@ def start_servers(default_store, coveragerc=None):
             "coverage run --rcfile={coveragerc} -m "
             "manage {service} --settings bok_choy runserver "
             "{address} --traceback --noreload".format(
-                default_store=default_store,
+                default_store=options.default_store,
                 coveragerc=coveragerc,
                 service=service,
                 address=address,
@@ -137,6 +142,8 @@ def is_mysql_running():
     return returncode == 0
 
 
+@task
+@timed
 def clear_mongo():
     """
     Clears mongo database.
@@ -148,6 +155,8 @@ def clear_mongo():
     )
 
 
+@task
+@timed
 def check_mongo():
     """
     Check that mongo is running
@@ -158,6 +167,8 @@ def check_mongo():
         sys.exit(1)
 
 
+@task
+@timed
 def check_memcache():
     """
     Check that memcache is running
@@ -168,6 +179,8 @@ def check_memcache():
         sys.exit(1)
 
 
+@task
+@timed
 def check_mysql():
     """
     Check that mysql is running
@@ -178,6 +191,8 @@ def check_mysql():
         sys.exit(1)
 
 
+@task
+@timed
 def check_services():
     """
     Check that all required services are running
diff --git a/pavelib/utils/test/suites/acceptance_suite.py b/pavelib/utils/test/suites/acceptance_suite.py
index e21f6088bd7..79d23eb862d 100644
--- a/pavelib/utils/test/suites/acceptance_suite.py
+++ b/pavelib/utils/test/suites/acceptance_suite.py
@@ -1,14 +1,70 @@
 """
 Acceptance test suite
 """
-from paver.easy import sh, call_task
+from paver.easy import sh, call_task, task
 from pavelib.utils.test import utils as test_utils
 from pavelib.utils.test.suites.suite import TestSuite
 from pavelib.utils.envs import Env
+from pavelib.utils.timer import timed
 
 __test__ = False  # do not collect
 
 
+DBS = {
+    'default': Env.REPO_ROOT / 'test_root/db/test_edx.db',
+    'student_module_history': Env.REPO_ROOT / 'test_root/db/test_student_module_history.db'
+}
+DB_CACHES = {
+    'default': Env.REPO_ROOT / 'common/test/db_cache/lettuce.db',
+    'student_module_history': Env.REPO_ROOT / 'common/test/db_cache/lettuce_student_module_history.db'
+}
+
+
+@task
+@timed
+def setup_acceptance_db():
+    """
+    TODO: Improve the following
+
+    Since the CMS depends on the existence of some database tables
+    that are now in common but used to be in LMS (Role/Permissions for Forums)
+    we need to create/migrate the database tables defined in the LMS.
+    We might be able to address this by moving out the migrations from
+    lms/django_comment_client, but then we'd have to repair all the existing
+    migrations from the upgrade tables in the DB.
+    But for now for either system (lms or cms), use the lms
+    definitions to sync and migrate.
+    """
+
+    for db in DBS.keys():
+        if DBS[db].isfile():
+            # Since we are using SQLLite, we can reset the database by deleting it on disk.
+            DBS[db].remove()
+
+    if all(DB_CACHES[cache].isfile() for cache in DB_CACHES.keys()):
+        # To speed up migrations, we check for a cached database file and start from that.
+        # The cached database file should be checked into the repo
+
+        # Copy the cached database to the test root directory
+        for db_alias in DBS.keys():
+            sh("cp {db_cache} {db}".format(db_cache=DB_CACHES[db_alias], db=DBS[db_alias]))
+
+        # Run migrations to update the db, starting from its cached state
+        for db_alias in sorted(DBS.keys()):
+            # pylint: disable=line-too-long
+            sh("./manage.py lms --settings acceptance migrate --traceback --noinput --fake-initial --database {}".format(db_alias))
+            sh("./manage.py cms --settings acceptance migrate --traceback --noinput --fake-initial --database {}".format(db_alias))
+    else:
+        # If no cached database exists, syncdb before migrating, then create the cache
+        for db_alias in sorted(DBS.keys()):
+            sh("./manage.py lms --settings acceptance migrate --traceback --noinput --database {}".format(db_alias))
+            sh("./manage.py cms --settings acceptance migrate --traceback --noinput --database {}".format(db_alias))
+
+        # Create the cache if it doesn't already exist
+        for db_alias in DBS.keys():
+            sh("cp {db} {db_cache}".format(db_cache=DB_CACHES[db_alias], db=DBS[db_alias]))
+
+
 class AcceptanceTest(TestSuite):
     """
     A class for running lettuce acceptance tests.
@@ -67,14 +123,6 @@ class AcceptanceTestSuite(TestSuite):
     def __init__(self, *args, **kwargs):
         super(AcceptanceTestSuite, self).__init__(*args, **kwargs)
         self.root = 'acceptance'
-        self.dbs = {
-            'default': Env.REPO_ROOT / 'test_root/db/test_edx.db',
-            'student_module_history': Env.REPO_ROOT / 'test_root/db/test_student_module_history.db'
-        }
-        self.db_caches = {
-            'default': Env.REPO_ROOT / 'common/test/db_cache/lettuce.db',
-            'student_module_history': Env.REPO_ROOT / 'common/test/db_cache/lettuce_student_module_history.db'
-        }
         self.fasttest = kwargs.get('fasttest', False)
 
         if kwargs.get('system'):
@@ -102,46 +150,4 @@ class AcceptanceTestSuite(TestSuite):
             test_utils.clean_test_files()
 
         if not self.fasttest:
-            self._setup_acceptance_db()
-
-    def _setup_acceptance_db(self):
-        """
-        TODO: Improve the following
-
-        Since the CMS depends on the existence of some database tables
-        that are now in common but used to be in LMS (Role/Permissions for Forums)
-        we need to create/migrate the database tables defined in the LMS.
-        We might be able to address this by moving out the migrations from
-        lms/django_comment_client, but then we'd have to repair all the existing
-        migrations from the upgrade tables in the DB.
-        But for now for either system (lms or cms), use the lms
-        definitions to sync and migrate.
-        """
-
-        for db in self.dbs.keys():
-            if self.dbs[db].isfile():
-                # Since we are using SQLLite, we can reset the database by deleting it on disk.
-                self.dbs[db].remove()
-
-        if all(self.db_caches[cache].isfile() for cache in self.db_caches.keys()):
-            # To speed up migrations, we check for a cached database file and start from that.
-            # The cached database file should be checked into the repo
-
-            # Copy the cached database to the test root directory
-            for db_alias in self.dbs.keys():
-                sh("cp {db_cache} {db}".format(db_cache=self.db_caches[db_alias], db=self.dbs[db_alias]))
-
-            # Run migrations to update the db, starting from its cached state
-            for db_alias in sorted(self.dbs.keys()):
-                # pylint: disable=line-too-long
-                sh("./manage.py lms --settings acceptance migrate --traceback --noinput --fake-initial --database {}".format(db_alias))
-                sh("./manage.py cms --settings acceptance migrate --traceback --noinput --fake-initial --database {}".format(db_alias))
-        else:
-            # If no cached database exists, syncdb before migrating, then create the cache
-            for db_alias in sorted(self.dbs.keys()):
-                sh("./manage.py lms --settings acceptance migrate --traceback --noinput --database {}".format(db_alias))
-                sh("./manage.py cms --settings acceptance migrate --traceback --noinput --database {}".format(db_alias))
-
-            # Create the cache if it doesn't already exist
-            for db_alias in self.dbs.keys():
-                sh("cp {db} {db_cache}".format(db_cache=self.db_caches[db_alias], db=self.dbs[db_alias]))
+            setup_acceptance_db()
diff --git a/pavelib/utils/test/suites/bokchoy_suite.py b/pavelib/utils/test/suites/bokchoy_suite.py
index 4929b589ea7..7618a503184 100644
--- a/pavelib/utils/test/suites/bokchoy_suite.py
+++ b/pavelib/utils/test/suites/bokchoy_suite.py
@@ -7,11 +7,15 @@ from urllib import urlencode
 from common.test.acceptance.fixtures.course import CourseFixture, FixtureError
 
 from path import Path as path
-from paver.easy import sh, BuildFailure
+from paver.easy import sh, BuildFailure, cmdopts, task, needs
 from pavelib.utils.test.suites.suite import TestSuite
 from pavelib.utils.envs import Env
-from pavelib.utils.test import bokchoy_utils
+from pavelib.utils.test.bokchoy_utils import (
+    clear_mongo, start_servers, check_services, wait_for_test_servers
+)
+from pavelib.utils.test.bokchoy_options import BOKCHOY_OPTS
 from pavelib.utils.test import utils as test_utils
+from pavelib.utils.timer import timed
 
 import os
 
@@ -26,6 +30,77 @@ DEFAULT_NUM_PROCESSES = 1
 DEFAULT_VERBOSITY = 2
 
 
+@task
+@cmdopts(BOKCHOY_OPTS, share_with=['test_bokchoy', 'test_a11y', 'pa11ycrawler'])
+@timed
+def load_bok_choy_data(options):
+    """
+    Loads data into database from db_fixtures
+    """
+    print 'Loading data from json fixtures in db_fixtures directory'
+    sh(
+        "DEFAULT_STORE={default_store}"
+        " ./manage.py lms --settings bok_choy loaddata --traceback"
+        " common/test/db_fixtures/*.json".format(
+            default_store=options.default_store,
+        )
+    )
+
+
+@task
+@cmdopts(BOKCHOY_OPTS, share_with=['test_bokchoy', 'test_a11y', 'pa11ycrawler'])
+@timed
+def load_courses(options):
+    """
+    Loads courses from options.imports_dir.
+
+    Note: options.imports_dir is the directory that contains the directories
+    that have courses in them. For example, if the course is located in
+    `test_root/courses/test-example-course/`, options.imports_dir should be
+    `test_root/courses/`.
+    """
+    if 'imports_dir' in options:
+        msg = colorize('green', "Importing courses from {}...".format(options.imports_dir))
+        print msg
+
+        sh(
+            "DEFAULT_STORE={default_store}"
+            " ./manage.py cms --settings=bok_choy import {import_dir}".format(
+                default_store=options.default_store,
+                import_dir=options.imports_dir
+            )
+        )
+
+
+@task
+@timed
+def reset_test_database():
+    """
+    Reset the database used by the bokchoy tests.
+    """
+    sh("{}/scripts/reset-test-db.sh".format(Env.REPO_ROOT))
+
+
+@task
+@needs(['reset_test_database', 'clear_mongo', 'load_bok_choy_data', 'load_courses'])
+@cmdopts(BOKCHOY_OPTS, share_with=['test_bokchoy', 'test_a11y', 'pa11ycrawler'])
+@timed
+def prepare_bokchoy_run(options, call_task):
+    """
+    Sets up and starts servers for a Bok Choy run. If --fasttest is not
+    specified then static assets are collected
+    """
+    if not options.get('fasttest', False):
+        print colorize('green', "Generating optimized static assets...")
+        # Use call_task so that we can specify options
+        call_task('update_assets', args=['--settings', 'test_static_optimized'])
+
+    # Ensure the test servers are available
+    msg = colorize('green', "Confirming servers are running...")
+    print msg
+    start_servers()  # pylint: disable=no-value-for-parameter
+
+
 class BokChoyTestSuite(TestSuite):
     """
     TestSuite for running Bok Choy tests
@@ -73,24 +148,24 @@ class BokChoyTestSuite(TestSuite):
         self.log_dir.makedirs_p()
         self.har_dir.makedirs_p()
         self.report_dir.makedirs_p()
-        test_utils.clean_reports_dir()      # pylint: disable=no-value-for-parameter
+        test_utils.clean_reports_dir()  # pylint: disable=no-value-for-parameter
 
         if not (self.fasttest or self.skip_clean or self.testsonly):
             test_utils.clean_test_files()
 
         msg = colorize('green', "Checking for mongo, memchache, and mysql...")
         print msg
-        bokchoy_utils.check_services()
+        check_services()
 
         if not self.testsonly:
-            self.prepare_bokchoy_run()
+            prepare_bokchoy_run()  # pylint: disable=no-value-for-parameter
         else:
             # load data in db_fixtures
-            self.load_data()
+            load_bok_choy_data()  # pylint: disable=no-value-for-parameter
 
         msg = colorize('green', "Confirming servers have started...")
         print msg
-        bokchoy_utils.wait_for_test_servers()
+        wait_for_test_servers()
         try:
             # Create course in order to seed forum data underneath. This is
             # a workaround for a race condition. The first time a course is created;
@@ -116,7 +191,7 @@ class BokChoyTestSuite(TestSuite):
             msg = colorize('green', "Cleaning up databases...")
             print msg
             sh("./manage.py lms --settings bok_choy flush --traceback --noinput")
-            bokchoy_utils.clear_mongo()
+            clear_mongo()
 
     @property
     def verbosity_processes_command(self):
@@ -147,66 +222,6 @@ class BokChoyTestSuite(TestSuite):
 
         return command
 
-    def prepare_bokchoy_run(self):
-        """
-        Sets up and starts servers for a Bok Choy run. If --fasttest is not
-        specified then static assets are collected
-        """
-        sh("{}/scripts/reset-test-db.sh".format(Env.REPO_ROOT))
-
-        if not self.fasttest:
-            self.generate_optimized_static_assets()
-
-        # Clear any test data already in Mongo or MySQLand invalidate
-        # the cache
-        bokchoy_utils.clear_mongo()
-        self.cache.flush_all()
-
-        # load data in db_fixtures
-        self.load_data()
-
-        # load courses if self.imports_dir is set
-        self.load_courses()
-
-        # Ensure the test servers are available
-        msg = colorize('green', "Confirming servers are running...")
-        print msg
-        bokchoy_utils.start_servers(self.default_store, self.coveragerc)
-
-    def load_courses(self):
-        """
-        Loads courses from self.imports_dir.
-
-        Note: self.imports_dir is the directory that contains the directories
-        that have courses in them. For example, if the course is located in
-        `test_root/courses/test-example-course/`, self.imports_dir should be
-        `test_root/courses/`.
-        """
-        msg = colorize('green', "Importing courses from {}...".format(self.imports_dir))
-        print msg
-
-        if self.imports_dir:
-            sh(
-                "DEFAULT_STORE={default_store}"
-                " ./manage.py cms --settings=bok_choy import {import_dir}".format(
-                    default_store=self.default_store,
-                    import_dir=self.imports_dir
-                )
-            )
-
-    def load_data(self):
-        """
-        Loads data into database from db_fixtures
-        """
-        print 'Loading data from json fixtures in db_fixtures directory'
-        sh(
-            "DEFAULT_STORE={default_store}"
-            " ./manage.py lms --settings bok_choy loaddata --traceback"
-            " common/test/db_fixtures/*.json".format(
-                default_store=self.default_store,
-            )
-        )
-
     def run_servers_continuously(self):
         """
         Infinite loop. Servers will continue to run in the current session unless interrupted.
diff --git a/pavelib/utils/test/suites/suite.py b/pavelib/utils/test/suites/suite.py
index dfe34a97c77..9c8e712ff8c 100644
--- a/pavelib/utils/test/suites/suite.py
+++ b/pavelib/utils/test/suites/suite.py
@@ -61,14 +61,6 @@ class TestSuite(object):
         """
         return None
 
-    def generate_optimized_static_assets(self):
-        """
-        Collect static assets using test_static_optimized.py which generates
-        optimized files to a dedicated test static root.
-        """
-        print colorize('green', "Generating optimized static assets...")
-        sh("paver update_assets --settings=test_static_optimized")
-
     def run_test(self):
         """
         Runs a self.cmd in a subprocess and waits for it to finish.
-- 
GitLab