Merge pull request #12794 from edx/benp/collectstatic-logging

FEDX-152. Pipe collectstatic results to a file for bok-choy tests.

pavelib/assets.py

@@ -64,6 +64,9 @@ SASS_LOOKUP_DEPENDENCIES = {
     'cms': [path('lms') / 'static' / 'sass' / 'partials', ],
 }
 
+# Collectstatic log directory setting
+COLLECTSTATIC_LOG_DIR_ARG = "collect_log_dir"
+
 
 def get_sass_directories(system, theme_dir=None):
     """
@@ -625,17 +628,42 @@ def restart_django_servers():
     ))
 
 
-def collect_assets(systems, settings):
+def collect_assets(systems, settings, **kwargs):
     """
     Collect static assets, including Django pipeline processing.
     `systems` is a list of systems (e.g. 'lms' or 'studio' or both)
     `settings` is the Django settings module to use.
+    `**kwargs` include arguments for using a log directory for collectstatic output. Defaults to /dev/null.
     """
+
     for sys in systems:
-        sh(django_cmd(sys, settings, "collectstatic --noinput > /dev/null"))
+        collectstatic_stdout_str = _collect_assets_cmd(sys, **kwargs)
+        sh(django_cmd(sys, settings, "collectstatic --noinput {logfile_str}".format(
+            logfile_str=collectstatic_stdout_str
+        )))
         print("\t\tFinished collecting {} assets.".format(sys))
 
 
+def _collect_assets_cmd(system, **kwargs):
+    """
+    Returns the collecstatic command to be used for the given system
+
+    Unless specified, collectstatic (which can be verbose) pipes to /dev/null
+    """
+    try:
+        if kwargs[COLLECTSTATIC_LOG_DIR_ARG] is None:
+            collectstatic_stdout_str = ""
+        else:
+            collectstatic_stdout_str = "> {output_dir}/{sys}-collectstatic.log".format(
+                output_dir=kwargs[COLLECTSTATIC_LOG_DIR_ARG],
+                sys=system
+            )
+    except KeyError:
+        collectstatic_stdout_str = "> /dev/null"
+
+    return collectstatic_stdout_str
+
+
 def execute_compile_sass(args):
     """
     Construct django management command compile_sass (defined in theming app) and execute it.
@@ -747,7 +775,12 @@ def update_assets(args):
         '--themes', type=str, nargs='+', default=None,
         help="list of themes to compile sass for",
     )
+    parser.add_argument(
+        '--collect-log', dest=COLLECTSTATIC_LOG_DIR_ARG, default=None,
+        help="When running collectstatic, direct output to specified log directory",
+    )
     args = parser.parse_args(args)
+    collect_log_args = {}
 
     process_xmodule_assets()
     process_npm_assets()
@@ -757,7 +790,13 @@ def update_assets(args):
     execute_compile_sass(args)
 
     if args.collect:
-        collect_assets(args.system, args.settings)
+        if args.debug:
+            collect_log_args.update({COLLECTSTATIC_LOG_DIR_ARG: None})
+
+        if args.collect_log_dir:
+            collect_log_args.update({COLLECTSTATIC_LOG_DIR_ARG: args.collect_log_dir})
+
+        collect_assets(args.system, args.settings, **collect_log_args)
 
     if args.watch:
         call_task(

pavelib/paver_tests/test_assets.py

@@ -3,6 +3,7 @@
 import ddt
 import os
 from unittest import TestCase
+from pavelib.assets import collect_assets, COLLECTSTATIC_LOG_DIR_ARG
 from paver.easy import call_task, path
 from mock import patch
 from watchdog.observers.polling import PollingObserver
@@ -202,3 +203,117 @@ class TestPaverWatchAssetTasks(TestCase):
                 self.assertIsInstance(sass_watcher_args[0], PollingObserver)
                 self.assertIsInstance(sass_watcher_args[1], list)
                 self.assertItemsEqual(sass_watcher_args[1], self.expected_sass_directories)
+
+
+@ddt.ddt
+class TestCollectAssets(PaverTestCase):
+    """
+    Test the collectstatic process call.
+
+    ddt data is organized thusly:
+      * debug: whether or not collect_assets is called with the debug flag
+      * specified_log_location: used when collect_assets is called with a specific
+          log location for collectstatic output
+      * expected_log_location: the expected string to be used for piping collectstatic logs
+    """
+
+    @ddt.data(
+        [{
+            "collect_log_args": {},  # Test for default behavior
+            "expected_log_location": "> /dev/null"
+        }],
+        [{
+            "collect_log_args": {COLLECTSTATIC_LOG_DIR_ARG: "/foo/bar"},
+            "expected_log_location": "> /foo/bar/lms-collectstatic.log"
+        }],  # can use specified log location
+        [{
+            "systems": ["lms", "cms"],
+            "collect_log_args": {},
+            "expected_log_location": "> /dev/null"
+        }],  # multiple systems can be called
+    )
+    @ddt.unpack
+    def test_collect_assets(self, options):
+        """
+        Ensure commands sent to the environment for collect_assets are as expected
+        """
+        specified_log_loc = options.get("collect_log_args", {})
+        specified_log_dict = specified_log_loc
+        log_loc = options.get("expected_log_location", "> /dev/null")
+        systems = options.get("systems", ["lms"])
+        expected_messages = self._set_expected_messages(log_location=log_loc, systems=systems)
+        if specified_log_loc is None:
+            collect_assets(
+                systems,
+                "devstack"
+            )
+        else:
+            collect_assets(
+                systems,
+                "devstack",
+                **specified_log_dict
+            )
+        self.assertEqual(self.task_messages, expected_messages)
+
+    def test_collect_assets_debug(self):
+        """
+        When the method is called specifically with None for the collectstatic log dir, then
+        it should run in debug mode and pipe to console.
+        """
+        expected_log_loc = ""
+        systems = ["lms"]
+        kwargs = {COLLECTSTATIC_LOG_DIR_ARG: None}
+        expected_messages = self._set_expected_messages(log_location=expected_log_loc, systems=systems)
+        collect_assets(systems, "devstack", **kwargs)
+        self.assertEqual(self.task_messages, expected_messages)
+
+    def _set_expected_messages(self, log_location, systems):
+        """
+        Returns a list of messages that are expected to be sent from paver
+         to the commandline for collectstatic functions. This list is constructed
+         based on the log location and systems being passed in.
+        """
+
+        expected_messages = []
+        for sys in systems:
+            expected_messages.append(
+                'python manage.py {system} --settings=devstack collectstatic --noinput {log_loc}'.format(
+                    system=sys,
+                    log_loc=log_location
+                )
+            )
+        return expected_messages
+
+
+@ddt.ddt
+class TestUpdateAssetsTask(PaverTestCase):
+    """
+    These are nearly end-to-end tests, because they observe output from the commandline request,
+    but do not actually execute the commandline on the terminal/process
+    """
+
+    @ddt.data(
+        [{"expected_substring": "> /dev/null"}],  # go to /dev/null by default
+        [{"cmd_args": ["--debug"], "expected_substring": "collectstatic --noinput "}]  # TODO: make this regex
+    )
+    @ddt.unpack
+    def test_update_assets_task_collectstatic_log_arg(self, options):
+        """
+        Scoped test that only looks at what is passed to the collecstatic options
+        """
+        cmd_args = options.get("cmd_args", [""])
+        expected_substring = options.get("expected_substring", None)
+        call_task('pavelib.assets.update_assets', args=cmd_args)
+        self.assertTrue(
+            self._is_substring_in_list(self.task_messages, expected_substring),
+            msg="{substring} not found in messages".format(substring=expected_substring)
+        )
+
+    def _is_substring_in_list(self, messages_list, expected_substring):
+        """
+        Return true a given string is somewhere in a list of strings
+        """
+        for message in messages_list:
+            if expected_substring in message:
+                return True
+        return False

pavelib/paver_tests/test_servers.py

@@ -29,7 +29,7 @@ EXPECTED_CMS_SASS_COMMAND = [
     u"python manage.py cms --settings={asset_settings} compile_sass cms ",
 ]
 EXPECTED_COLLECT_STATIC_COMMAND = (
-    u"python manage.py {system} --settings={asset_settings} collectstatic --noinput > /dev/null"
+    u"python manage.py {system} --settings={asset_settings} collectstatic --noinput {log_string}"
 )
 EXPECTED_CELERY_COMMAND = (
     u"python manage.py lms --settings={settings} celery worker --beat --loglevel=INFO --pythonpath=."
@@ -193,6 +193,7 @@ class TestPaverServerTasks(PaverTestCase):
         """
         Verify the output of a server task.
         """
+        log_string = options.get("log_string", "> /dev/null")
         settings = options.get("settings", None)
         asset_settings = options.get("asset-settings", None)
         is_optimized = options.get("optimized", False)
@@ -235,7 +236,7 @@ class TestPaverServerTasks(PaverTestCase):
             expected_messages.extend(self.expected_sass_commands(system=system, asset_settings=expected_asset_settings))
         if expected_collect_static:
             expected_messages.append(EXPECTED_COLLECT_STATIC_COMMAND.format(
-                system=system, asset_settings=expected_asset_settings
+                system=system, asset_settings=expected_asset_settings, log_string=log_string
             ))
         expected_run_server_command = EXPECTED_RUN_SERVER_COMMAND.format(
             system=system,
@@ -251,6 +252,7 @@ class TestPaverServerTasks(PaverTestCase):
         """
         Verify the output of a server task.
         """
+        log_string = options.get("log_string", "> /dev/null")
         settings = options.get("settings", None)
         asset_settings = options.get("asset_settings", None)
         is_optimized = options.get("optimized", False)
@@ -271,10 +273,10 @@ class TestPaverServerTasks(PaverTestCase):
             expected_messages.extend(self.expected_sass_commands(asset_settings=expected_asset_settings))
         if expected_collect_static:
             expected_messages.append(EXPECTED_COLLECT_STATIC_COMMAND.format(
-                system="lms", asset_settings=expected_asset_settings
+                system="lms", asset_settings=expected_asset_settings, log_string=log_string
             ))
             expected_messages.append(EXPECTED_COLLECT_STATIC_COMMAND.format(
-                system="cms", asset_settings=expected_asset_settings
+                system="cms", asset_settings=expected_asset_settings, log_string=log_string
             ))
         expected_messages.append(
             EXPECTED_RUN_SERVER_COMMAND.format(

pavelib/utils/test/suites/bokchoy_suite.py

@@ -155,7 +155,7 @@ class BokChoyTestSuite(TestSuite):
         sh("{}/scripts/reset-test-db.sh".format(Env.REPO_ROOT))
 
         if not self.fasttest:
-            self.generate_optimized_static_assets()
+            self.generate_optimized_static_assets(log_dir=self.log_dir)
 
         # Clear any test data already in Mongo or MySQLand invalidate
         # the cache

pavelib/utils/test/suites/suite.py

@@ -61,13 +61,19 @@ class TestSuite(object):
         """
         return None
 
-    def generate_optimized_static_assets(self):
+    def generate_optimized_static_assets(self, log_dir=None):
         """
         Collect static assets using test_static_optimized.py which generates
-        optimized files to a dedicated test static root.
+        optimized files to a dedicated test static root. Optionally use
+        a log directory for collectstatic output.
         """
         print colorize('green', "Generating optimized static assets...")
-        sh("paver update_assets --settings=test_static_optimized")
+        if not log_dir:
+            sh("paver update_assets --settings=test_static_optimized")
+        else:
+            sh("paver update_assets --settings=test_static_optimized --collect-log={log_dir}".format(
+                log_dir=log_dir
+            ))
 
     def run_test(self):
         """