From 066450d2742d9de6ca18bc3da7c84839ea704750 Mon Sep 17 00:00:00 2001 From: Ben Patterson Date: Fri, 17 Jun 2016 12:49:08 -0400 Subject: [PATCH] FEDX-152. Pipe collectstatic results to a file for bok-choy tests. --- pavelib/assets.py | 45 +++++++- pavelib/paver_tests/test_assets.py | 115 +++++++++++++++++++++ pavelib/paver_tests/test_servers.py | 10 +- pavelib/utils/test/suites/bokchoy_suite.py | 2 +- pavelib/utils/test/suites/suite.py | 12 ++- 5 files changed, 173 insertions(+), 11 deletions(-) diff --git a/pavelib/assets.py b/pavelib/assets.py index 3bb5d81454..7c96900b75 100644 --- a/pavelib/assets.py +++ b/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( diff --git a/pavelib/paver_tests/test_assets.py b/pavelib/paver_tests/test_assets.py index 37f105993a..26c6726080 100644 --- a/pavelib/paver_tests/test_assets.py +++ b/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 diff --git a/pavelib/paver_tests/test_servers.py b/pavelib/paver_tests/test_servers.py index e2c7f9cbd5..bed8acf5e0 100644 --- a/pavelib/paver_tests/test_servers.py +++ b/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( diff --git a/pavelib/utils/test/suites/bokchoy_suite.py b/pavelib/utils/test/suites/bokchoy_suite.py index 4929b589ea..c594661cd6 100644 --- a/pavelib/utils/test/suites/bokchoy_suite.py +++ b/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 diff --git a/pavelib/utils/test/suites/suite.py b/pavelib/utils/test/suites/suite.py index dfe34a97c7..b5b6494478 100644 --- a/pavelib/utils/test/suites/suite.py +++ b/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): """