diff --git a/cms/envs/common.py b/cms/envs/common.py index 6aa2dfb835b1..39d5c7d13251 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -39,7 +39,6 @@ # pylint: disable=unused-import, useless-suppression, wrong-import-order, wrong-import-position import importlib.util -import json import os import sys @@ -1276,11 +1275,14 @@ # Static content STATIC_URL = '/static/studio/' -STATIC_ROOT = os.environ.get('STATIC_ROOT_CMS', ENV_ROOT / 'staticfiles' / 'studio') +STATIC_ROOT = ENV_ROOT / "staticfiles" / 'studio' STATICFILES_DIRS = [ COMMON_ROOT / "static", PROJECT_ROOT / "static", + + # This is how you would use the textbook images locally + # ("book", ENV_ROOT / "book_images"), ] # Locale/Internationalization @@ -1320,16 +1322,11 @@ EMBARGO_SITE_REDIRECT_URL = None ##### custom vendor plugin variables ##### - -# .. setting_name: JS_ENV_EXTRA_CONFIG -# .. setting_default: {} -# .. setting_description: JavaScript code can access this dictionary using `process.env.JS_ENV_EXTRA_CONFIG` -# One of the current use cases for this is enabling custom TinyMCE plugins -# (TINYMCE_ADDITIONAL_PLUGINS) and overriding the TinyMCE configuration (TINYMCE_CONFIG_OVERRIDES). -# .. setting_warning: This Django setting is DEPRECATED! Starting in Sumac, Webpack will no longer -# use Django settings. Please set the JS_ENV_EXTRA_CONFIG environment variable to an equivalent JSON -# string instead. For details, see: https://github.com/openedx/edx-platform/issues/31895 -JS_ENV_EXTRA_CONFIG = json.loads(os.environ.get('JS_ENV_EXTRA_CONFIG', '{}')) +# JavaScript code can access this data using `process.env.JS_ENV_EXTRA_CONFIG` +# One of the current use cases for this is enabling custom TinyMCE plugins +# (TINYMCE_ADDITIONAL_PLUGINS) and overriding the TinyMCE configuration +# (TINYMCE_CONFIG_OVERRIDES). +JS_ENV_EXTRA_CONFIG = {} ############################### PIPELINE ####################################### @@ -1506,14 +1503,7 @@ 'STATS_FILE': os.path.join(STATIC_ROOT, 'webpack-worker-stats.json') } } - -# .. setting_name: WEBPACK_CONFIG_PATH -# .. setting_default: "webpack.prod.config.js" -# .. setting_description: Path to the Webpack configuration file. Used by Paver scripts. -# .. setting_warning: This Django setting is DEPRECATED! Starting in Sumac, Webpack will no longer -# use Django settings. Please set the WEBPACK_CONFIG_PATH environment variable instead. For details, -# see: https://github.com/openedx/edx-platform/issues/31895 -WEBPACK_CONFIG_PATH = os.environ.get('WEBPACK_CONFIG_PATH', 'webpack.prod.config.js') +WEBPACK_CONFIG_PATH = 'webpack.prod.config.js' ############################ SERVICE_VARIANT ################################## @@ -2190,11 +2180,8 @@ # .. setting_name: COMPREHENSIVE_THEME_DIRS # .. setting_default: [] -# .. setting_description: A list of paths to directories, each of which will -# be searched for comprehensive themes. Do not override this Django setting directly. -# Instead, set the COMPREHENSIVE_THEME_DIRS environment variable, using colons (:) to -# separate paths. -COMPREHENSIVE_THEME_DIRS = os.environ.get("COMPREHENSIVE_THEME_DIRS", "").split(":") +# .. setting_description: See LMS annotation. +COMPREHENSIVE_THEME_DIRS = [] # .. setting_name: COMPREHENSIVE_THEME_LOCALE_PATHS # .. setting_default: [] diff --git a/docs/decisions/0017-reimplement-asset-processing.rst b/docs/decisions/0017-reimplement-asset-processing.rst index e689c34eb9cf..1048edf4c075 100644 --- a/docs/decisions/0017-reimplement-asset-processing.rst +++ b/docs/decisions/0017-reimplement-asset-processing.rst @@ -11,14 +11,16 @@ Overview Status ****** -**Accepted** +**Provisional** -This was `originally authored `_ in March 2023. We `modified it in July 2023 `_ based on learnings from the implementation process, and then `modified and it again in May 2024 `_ to make the migration easier for operators to understand. +This was `originally authored `_ in March 2023. We `modified it in July 2023 `_ based on learnings from the implementation process. -Related deprecation tickets: +The status will be moved to *Accepted* upon completion of reimplementation. Related work: * `[DEPR]: Asset processing in Paver `_ -* `[DEPR]: Paver `_ +* `Process edx-platform assets without Paver `_ +* `Process edx-platform assets without Python `_ + Context ******* @@ -90,6 +92,7 @@ Three particular issues have surfaced in Developer Experience Working Group disc All of these potential solutions would involve refactoring or entirely replacing parts of the current asset processing system. + Decision ******** @@ -111,9 +114,6 @@ Reimplementation Specification Commands and stages ------------------- -**May 2024 update:** See the `static assets reference <../references/static-assets.rst>`_ for -the latest commands. - The three top-level edx-platform asset processing actions are *build*, *collect*, and *watch*. The build action can be further broken down into five stages. Here is how those actions and stages will be reimplemented: @@ -226,9 +226,6 @@ The three top-level edx-platform asset processing actions are *build*, *collect* Build Configuration ------------------- -**May 2024 update:** See the `static assets reference <../references/static-assets.rst>`_ for -the latest configuration settings. - To facilitate a generally Python-free build reimplementation, we will require that certain Django settings now be specified as environment variables, which can be passed to the build like so:: MY_ENV_VAR="my value" npm run build # Set for the whole build. @@ -269,7 +266,7 @@ Some of these options will remain as Django settings because they are used in ed * - ``COMPREHENSIVE_THEME_DIRS`` - Directories that will be searched when compiling themes. - ``COMPREHENSIVE_THEME_DIRS`` - - ``COMPREHENSIVE_THEME_DIRS`` + - ``EDX_PLATFORM_THEME_DIRS`` Migration ========= @@ -288,16 +285,37 @@ As a consequence of this ADR, Tutor will either need to: * reimplement the script as a thin wrapper around the new asset processing commands, or * deprecate and remove the script. -**May 2024 update:** The ``openedx-assets`` script will be removed from Tutor, -with migration instructions documented in -`Tutor's changelog `_. +Either way, the migration path is straightforward: + +.. list-table:: + :header-rows: 1 + + * - Existing Tutor-provided command + - New upstream command + * - ``openedx-assets build`` + - ``npm run build`` + * - ``openedx-assets npm`` + - ``scripts/copy-node-modules.sh # (automatically invoked by 'npm install'!)`` + * - ``openedx-assets xmodule`` + - (no longer needed) + * - ``openedx-assets common`` + - ``npm run compile-sass -- --skip-themes`` + * - ``openedx-assets themes`` + - ``npm run compile-sass -- --skip-default`` + * - ``openedx-assets webpack`` + - ``npm run webpack`` + * - ``openedx-assets collect`` + - ``./manage.py [lms|cms] collectstatic --noinput`` + * - ``openedx-assets watch-themes`` + - ``npm run watch`` + +The options accepted by ``openedx-assets`` will all be valid inputs to ``scripts/build-assets.sh``. non-Tutor migration guide ------------------------- -A migration guide for site operators who are directly referencing Paver will be -included in the -`Paver deprecation ticket `_. +Operators using distributions other than Tutor should refer to the upstream edx-platform changes described above in **Reimplementation Specification**, and adapt them accordingly to their distribution. + See also ******** diff --git a/lms/envs/common.py b/lms/envs/common.py index 02e52ea5a317..f6266fdaa3cb 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1928,7 +1928,7 @@ def _make_mako_template_dirs(settings): # Static content STATIC_URL = '/static/' -STATIC_ROOT = os.environ.get('STATIC_ROOT_LMS', ENV_ROOT / "staticfiles") +STATIC_ROOT = ENV_ROOT / "staticfiles" STATIC_URL_BASE = '/static/' STATICFILES_DIRS = [ @@ -2822,14 +2822,7 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring 'STATS_FILE': os.path.join(STATIC_ROOT, 'webpack-worker-stats.json') } } - -# .. setting_name: WEBPACK_CONFIG_PATH -# .. setting_default: "webpack.prod.config.js" -# .. setting_description: Path to the Webpack configuration file. Used by Paver scripts. -# .. setting_warning: This Django setting is DEPRECATED! Starting in Sumac, Webpack will no longer -# use Django settings. Please set the WEBPACK_CONFIG_PATH environment variable instead. For details, -# see: https://github.com/openedx/edx-platform/issues/31895 -WEBPACK_CONFIG_PATH = os.environ.get('WEBPACK_CONFIG_PATH', 'webpack.prod.config.js') +WEBPACK_CONFIG_PATH = 'webpack.prod.config.js' ########################## DJANGO DEBUG TOOLBAR ############################### @@ -4556,11 +4549,9 @@ def _make_locale_paths(settings): # pylint: disable=missing-function-docstring # .. setting_name: COMPREHENSIVE_THEME_DIRS # .. setting_default: [] -# .. setting_description: A list of paths to directories, each of which will -# be searched for comprehensive themes. Do not override this Django setting directly. -# Instead, set the COMPREHENSIVE_THEME_DIRS environment variable, using colons (:) to -# separate paths. -COMPREHENSIVE_THEME_DIRS = os.environ.get("COMPREHENSIVE_THEME_DIRS", "").split(":") +# .. setting_description: A list of directories containing themes folders, +# each entry should be a full path to the directory containing the theme folder. +COMPREHENSIVE_THEME_DIRS = [] # .. setting_name: COMPREHENSIVE_THEME_LOCALE_PATHS # .. setting_default: [] diff --git a/openedx/core/djangoapps/theming/management/commands/compile_sass.py b/openedx/core/djangoapps/theming/management/commands/compile_sass.py index eecbe684b528..765ef98aeac3 100644 --- a/openedx/core/djangoapps/theming/management/commands/compile_sass.py +++ b/openedx/core/djangoapps/theming/management/commands/compile_sass.py @@ -1,14 +1,13 @@ """ Management command for compiling sass. - -DEPRECATED in favor of `npm run compile-sass`. """ -import shlex -from django.core.management import BaseCommand -from django.conf import settings -from pavelib.assets import run_deprecated_command_wrapper +from django.core.management import BaseCommand, CommandError +from paver.easy import call_task + +from openedx.core.djangoapps.theming.helpers import get_theme_base_dirs, get_themes, is_comprehensive_theming_enabled +from pavelib.assets import ALL_SYSTEMS class Command(BaseCommand): @@ -16,7 +15,7 @@ class Command(BaseCommand): Compile theme sass and collect theme assets. """ - help = "DEPRECATED. Use 'npm run compile-sass' instead." + help = 'Compile and collect themed assets...' # NOTE (CCB): This allows us to compile static assets in Docker containers without database access. requires_system_checks = [] @@ -29,7 +28,7 @@ def add_arguments(self, parser): parser (django.core.management.base.CommandParser): parsed for parsing command line arguments. """ parser.add_argument( - 'system', type=str, nargs='*', default=["lms", "cms"], + 'system', type=str, nargs='*', default=ALL_SYSTEMS, help="lms or studio", ) @@ -56,7 +55,7 @@ def add_arguments(self, parser): '--force', action='store_true', default=False, - help="DEPRECATED. Full recompilation is now always forced.", + help="Force full compilation", ) parser.add_argument( '--debug', @@ -65,48 +64,77 @@ def add_arguments(self, parser): help="Disable Sass compression", ) - def handle(self, *args, **options): + @staticmethod + def parse_arguments(*args, **options): # pylint: disable=unused-argument """ - Handle compile_sass command. + Parse and validate arguments for compile_sass command. + + Args: + *args: Positional arguments passed to the update_assets command + **options: optional arguments passed to the update_assets command + Returns: + A tuple containing parsed values for themes, system, source comments and output style. + 1. system (list): list of system names for whom to compile theme sass e.g. 'lms', 'cms' + 2. theme_dirs (list): list of Theme objects + 3. themes (list): list of Theme objects + 4. force (bool): Force full compilation + 5. debug (bool): Disable Sass compression """ - systems = set( - {"lms": "lms", "cms": "cms", "studio": "cms"}[sys] - for sys in options.get("system", ["lms", "cms"]) - ) - theme_dirs = options.get("theme_dirs", settings.COMPREHENSIVE_THEME_DIRS) or [] - themes_option = options.get("themes", []) # '[]' means 'all' - if not settings.ENABLE_COMPREHENSIVE_THEMING: - compile_themes = False - themes = [] - elif "no" in themes_option: - compile_themes = False - themes = [] - elif "all" in themes_option: - compile_themes = True - themes = [] + system = options.get("system", ALL_SYSTEMS) + given_themes = options.get("themes", ["all"]) + theme_dirs = options.get("theme_dirs", None) + + force = options.get("force", True) + debug = options.get("debug", True) + + if theme_dirs: + available_themes = {} + for theme_dir in theme_dirs: + available_themes.update({t.theme_dir_name: t for t in get_themes(theme_dir)}) else: - compile_themes = True - themes = themes_option - run_deprecated_command_wrapper( - old_command="./manage.py [lms|cms] compile_sass", - ignored_old_flags=list(set(["force"]) & set(options)), - new_command=shlex.join([ - "npm", - "run", - ("compile-sass-dev" if options.get("debug") else "compile-sass"), - "--", - *(["--skip-lms"] if "lms" not in systems else []), - *(["--skip-cms"] if "cms" not in systems else []), - *(["--skip-themes"] if not compile_themes else []), - *( - arg - for theme_dir in theme_dirs - for arg in ["--theme-dir", str(theme_dir)] + theme_dirs = get_theme_base_dirs() + available_themes = {t.theme_dir_name: t for t in get_themes()} + + if 'no' in given_themes or 'all' in given_themes: + # Raise error if 'all' or 'no' is present and theme names are also given. + if len(given_themes) > 1: + raise CommandError("Invalid themes value, It must either be 'all' or 'no' or list of themes.") + # Raise error if any of the given theme name is invalid + # (theme name would be invalid if it does not exist in themes directory) + elif (not set(given_themes).issubset(list(available_themes.keys()))) and is_comprehensive_theming_enabled(): + raise CommandError( + "Given themes '{themes}' do not exist inside any of the theme directories '{theme_dirs}'".format( + themes=", ".join(set(given_themes) - set(available_themes.keys())), + theme_dirs=theme_dirs, ), - *( - arg - for theme in themes - for arg in ["--theme", theme] + ) + + if "all" in given_themes: + themes = list(available_themes.values()) + elif "no" in given_themes: + themes = [] + else: + # convert theme names to Theme objects, this will remove all themes if theming is disabled + themes = [available_themes.get(theme) for theme in given_themes if theme in available_themes] + + return system, theme_dirs, themes, force, debug + + def handle(self, *args, **options): + """ + Handle compile_sass command. + """ + system, theme_dirs, themes, force, debug = self.parse_arguments(*args, **options) + themes = [theme.theme_dir_name for theme in themes] + + if options.get("themes", None) and not is_comprehensive_theming_enabled(): + # log a warning message to let the user know that asset compilation for themes is skipped + self.stdout.write( + self.style.WARNING( + "Skipping theme asset compilation: enable theming to process themed assets" ), - ]), + ) + + call_task( + 'pavelib.assets.compile_sass', + options={'system': system, 'theme_dirs': theme_dirs, 'themes': themes, 'force': force, 'debug': debug}, ) diff --git a/openedx/core/djangoapps/theming/tests/test_commands.py b/openedx/core/djangoapps/theming/tests/test_commands.py index 7ca56f604a41..b4abee9bed0e 100644 --- a/openedx/core/djangoapps/theming/tests/test_commands.py +++ b/openedx/core/djangoapps/theming/tests/test_commands.py @@ -2,23 +2,57 @@ Tests for Management commands of comprehensive theming. """ -from django.core.management import call_command -from django.test import TestCase, override_settings -from unittest.mock import patch +import pytest +from django.core.management import CommandError, call_command +from django.test import TestCase -import pavelib.assets +from openedx.core.djangoapps.theming.helpers import get_themes +from openedx.core.djangoapps.theming.management.commands.compile_sass import Command class TestUpdateAssets(TestCase): """ Test comprehensive theming helper functions. """ + def setUp(self): + super().setUp() + self.themes = get_themes() - @patch.object(pavelib.assets, 'sh') - @override_settings(COMPREHENSIVE_THEME_DIRS='common/test') - def test_deprecated_wrapper(self, mock_sh): - call_command('compile_sass', '--themes', 'fake-theme1', 'fake-theme2') - assert mock_sh.called_once_with( - "npm run compile-sass -- " + - "--theme-dir common/test --theme fake-theme-1 --theme fake-theme-2" + def test_errors_for_invalid_arguments(self): + """ + Test update_asset command. + """ + # make sure error is raised for invalid theme list + with pytest.raises(CommandError): + call_command("compile_sass", themes=["all", "test-theme"]) + + # make sure error is raised for invalid theme list + with pytest.raises(CommandError): + call_command("compile_sass", themes=["no", "test-theme"]) + + # make sure error is raised for invalid theme list + with pytest.raises(CommandError): + call_command("compile_sass", themes=["all", "no"]) + + # make sure error is raised for invalid theme list + with pytest.raises(CommandError): + call_command("compile_sass", themes=["test-theme", "non-existing-theme"]) + + def test_parse_arguments(self): + """ + Test parse arguments method for update_asset command. + """ + # make sure compile_sass picks all themes when called with 'themes=all' option + parsed_args = Command.parse_arguments(themes=["all"]) + self.assertCountEqual(parsed_args[2], get_themes()) + + # make sure compile_sass picks no themes when called with 'themes=no' option + parsed_args = Command.parse_arguments(themes=["no"]) + self.assertCountEqual(parsed_args[2], []) + + # make sure compile_sass picks only specified themes + parsed_args = Command.parse_arguments(themes=["test-theme"]) + self.assertCountEqual( + parsed_args[2], + [theme for theme in get_themes() if theme.theme_dir_name == "test-theme"] ) diff --git a/pavelib/assets.py b/pavelib/assets.py index f437b6427f93..466ffc9ed919 100644 --- a/pavelib/assets.py +++ b/pavelib/assets.py @@ -1,9 +1,5 @@ """ Asset compilation and collection. - -This entire module is DEPRECATED. In Redwood, it exists just as a collection of temporary compatibility wrappers. -In Sumac, this module will be deleted. To migrate, follow the advice in the printed warnings and/or read the -instructions on the DEPR ticket: https://github.com/openedx/edx-platform/issues/31895 """ import argparse @@ -17,48 +13,31 @@ from paver import tasks from paver.easy import call_task, cmdopts, consume_args, needs, no_help, sh, task from watchdog.events import PatternMatchingEventHandler -from watchdog.observers import Observer # pylint: disable=unused-import # Used by Tutor. Remove after Sumac cut. +from watchdog.observers import Observer # pylint disable=unused-import # Used by Tutor. Remove after Sumac cut. -from .utils.cmd import django_cmd +from .utils.cmd import cmd, django_cmd from .utils.envs import Env +from .utils.process import run_background_process from .utils.timer import timed +# setup baseline paths + +ALL_SYSTEMS = ['lms', 'studio'] + +LMS = 'lms' +CMS = 'cms' SYSTEMS = { - 'lms': 'lms', - 'cms': 'cms', - 'studio': 'cms', + 'lms': LMS, + 'cms': CMS, + 'studio': CMS } -WARNING_SYMBOLS = "⚠️ " * 50 # A row of 'warning' emoji to catch CLI users' attention - +# Collectstatic log directory setting +COLLECTSTATIC_LOG_DIR_ARG = 'collect_log_dir' -def run_deprecated_command_wrapper(*, old_command, ignored_old_flags, new_command): - """ - Run the new version of shell command, plus a warning that the old version is deprecated. - """ - depr_warning = ( - "\n" + - f"{WARNING_SYMBOLS}\n" + - "\n" + - f"WARNING: '{old_command}' is DEPRECATED! It will be removed before Sumac.\n" + - "The command you ran is now just a temporary wrapper around a new,\n" + - "supported command, which you should use instead:\n" + - "\n" + - f"\t{new_command}\n" + - "\n" + - "Details: https://github.com/openedx/edx-platform/issues/31895\n" + - "".join( - f" WARNING: ignored deprecated paver flag '{flag}'\n" - for flag in ignored_old_flags - ) + - f"{WARNING_SYMBOLS}\n" + - "\n" - ) - # Print deprecation warning twice so that it's more likely to be seen in the logs. - print(depr_warning) - sh(new_command) - print(depr_warning) +# Webpack command +WEBPACK_COMMAND = 'STATIC_ROOT_LMS={static_root_lms} STATIC_ROOT_CMS={static_root_cms} webpack {options}' def debounce(seconds=1): @@ -66,8 +45,6 @@ def debounce(seconds=1): Prevents the decorated function from being called more than every `seconds` seconds. Waits until calls stop coming in before calling the decorated function. - - This is DEPRECATED. It exists in Redwood just to ease the transition for Tutor. """ def decorator(func): func.timer = None @@ -89,8 +66,6 @@ def call(): class SassWatcher(PatternMatchingEventHandler): """ Watches for sass file changes - - This is DEPRECATED. It exists in Redwood just to ease the transition for Tutor. """ ignore_directories = True patterns = ['*.scss'] @@ -127,7 +102,7 @@ def on_any_event(self, event): ('system=', 's', 'The system to compile sass for (defaults to all)'), ('theme-dirs=', '-td', 'Theme dirs containing all themes (defaults to None)'), ('themes=', '-t', 'The theme to compile sass for (defaults to None)'), - ('debug', 'd', 'Whether to use development settings'), + ('debug', 'd', 'DEPRECATED. Debug mode is now determined by NODE_ENV.'), ('force', '', 'DEPRECATED. Full recompilation is now always forced.'), ]) @timed @@ -168,18 +143,16 @@ def compile_sass(options): This is a DEPRECATED COMPATIBILITY WRAPPER. Use `npm run compile-sass` instead. """ - systems = [SYSTEMS[sys] for sys in get_parsed_option(options, 'system', ['lms', 'cms'])] # normalize studio->cms - run_deprecated_command_wrapper( - old_command="paver compile_sass", - ignored_old_flags=(set(["--force"]) & set(options)), - new_command=shlex.join([ + systems = set(get_parsed_option(options, 'system', ALL_SYSTEMS)) + command = shlex.join( + [ "npm", "run", - ("compile-sass-dev" if options.get("debug") else "compile-sass"), + "compile-sass", "--", *(["--dry"] if tasks.environment.dry_run else []), - *(["--skip-lms"] if "lms" not in systems else []), - *(["--skip-cms"] if "cms" not in systems else []), + *(["--skip-lms"] if not systems & {"lms"} else []), + *(["--skip-cms"] if not systems & {"cms", "studio"} else []), *( arg for theme_dir in get_parsed_option(options, 'theme_dirs', []) @@ -187,50 +160,77 @@ def compile_sass(options): ), *( arg - for theme in get_parsed_option(options, "themes", []) + for theme in get_parsed_option(options, "theme", []) for arg in ["--theme", theme] ), - ]), + ] + ) + depr_warning = ( + "\n" + + "⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ \n" + + "\n" + + "WARNING: 'paver compile_sass' is DEPRECATED! It will be removed before Sumac.\n" + + "The command you ran is now just a temporary wrapper around a new,\n" + + "supported command, which you should use instead:\n" + + "\n" + + f"\t{command}\n" + + "\n" + + "Details: https://github.com/openedx/edx-platform/issues/31895\n" + + "\n" + + ("WARNING: ignoring deprecated flag '--debug'\n" if options.get("debug") else "") + + ("WARNING: ignoring deprecated flag '--force'\n" if options.get("force") else "") + + "⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ \n" + + "\n" ) + # Print deprecation warning twice so that it's more likely to be seen in the logs. + print(depr_warning) + sh(command) + print(depr_warning) -def _compile_sass(system, theme, debug, force, _timing_info): +def _compile_sass(system, theme, _debug, _force, _timing_info): """ This is a DEPRECATED COMPATIBILITY WRAPPER It exists to ease the transition for Tutor in Redwood, which directly imported and used this function. """ - run_deprecated_command_wrapper( - old_command="pavelib.assets:_compile_sass", - ignored_old_flags=(set(["--force"]) if force else set()), - new_command=[ + command = shlex.join( + [ "npm", "run", - ("compile-sass-dev" if debug else "compile-sass"), + "compile-sass", "--", *(["--dry"] if tasks.environment.dry_run else []), - *( - ["--skip-default", "--theme-dir", str(theme.parent), "--theme", str(theme.name)] - if theme - else [] - ), + *(["--skip-default", "--theme-dir", str(theme.parent), "--theme", str(theme.name)] if theme else []), ("--skip-cms" if system == "lms" else "--skip-lms"), ] ) + depr_warning = ( + "\n" + + "⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ \n" + + "\n" + + "WARNING: 'pavelib/assets.py' is DEPRECATED! It will be removed before Sumac.\n" + + "The function you called is just a temporary wrapper around a new, supported command,\n" + + "which you should use instead:\n" + + "\n" + + f"\t{command}\n" + + "\n" + + "Details: https://github.com/openedx/edx-platform/issues/31895\n" + + "\n" + + "⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ \n" + + "\n" + ) + # Print deprecation warning twice so that it's more likely to be seen in the logs. + print(depr_warning) + sh(command) + print(depr_warning) def process_npm_assets(): """ Process vendor libraries installed via NPM. - - This is a DEPRECATED COMPATIBILITY WRAPPER. It is now handled as part of `npm clean-install`. - If you need to invoke it explicitly, you can run `npm run postinstall`. """ - run_deprecated_command_wrapper( - old_command="pavelib.assets:process_npm_assets", - ignored_old_flags=[], - new_command=shlex.join(["npm", "run", "postinstall"]), - ) + sh('scripts/copy-node-modules.sh') @task @@ -238,21 +238,9 @@ def process_npm_assets(): def process_xmodule_assets(): """ Process XModule static assets. - - This is a DEPRECATED COMPATIBILITY STUB. Refrences to it should be deleted. """ - print( - "\n" + - f"{WARNING_SYMBOLS}", - "\n" + - "WARNING: 'paver process_xmodule_assets' is DEPRECATED! It will be removed before Sumac.\n" + - "\n" + - "Starting with Quince, it is no longer necessary to post-process XModule assets, so \n" + - "'paver process_xmodule_assets' is a no-op. Please simply remove it from your build scripts.\n" + - "\n" + - "Details: https://github.com/openedx/edx-platform/issues/31895\n" + - f"{WARNING_SYMBOLS}", - ) + print("\t\tProcessing xmodule assets is no longer needed. This task is now a no-op.") + print("\t\tWhen paver is removed from edx-platform, this step will not replaced.") def collect_assets(systems, settings, **kwargs): @@ -261,29 +249,33 @@ def collect_assets(systems, settings, **kwargs): `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: + collectstatic_stdout_str = _collect_assets_cmd(sys, **kwargs) + sh(django_cmd(sys, settings, "collectstatic --noinput {logfile_str}".format( + logfile_str=collectstatic_stdout_str + ))) + print(f"\t\tFinished collecting {sys} assets.") - This is a DEPRECATED COMPATIBILITY WRAPPER - It exists to ease the transition for Tutor in Redwood, which directly imported and used this function. +def _collect_assets_cmd(system, **kwargs): """ - run_deprecated_command_wrapper( - old_command="pavelib.asset:collect_assets", - ignored_old_flags=[], - new_command=" && ".join( - "( " + - shlex.join( - ["./manage.py", SYSTEMS[sys], f"--settings={settings}", "collectstatic", "--noinput"] - ) + ( - "" - if "collect_log_dir" not in kwargs else - " > /dev/null" - if kwargs["collect_log_dir"] is None else - f"> {kwargs['collect_log_dir']}/{SYSTEMS[sys]}-collectstatic.out" - ) + - " )" - for sys in systems - ), - ) + 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): @@ -291,8 +283,6 @@ def execute_compile_sass(args): Construct django management command compile_sass (defined in theming app) and execute it. Args: args: command line argument passed via update_assets command - - This is a DEPRECATED COMPATIBILITY WRAPPER. Use `npm run compile-sass` instead. """ for sys in args.system: options = "" @@ -315,14 +305,12 @@ def execute_compile_sass(args): @task @cmdopts([ ('settings=', 's', "Django settings (defaults to devstack)"), - ('watch', 'w', "DEPRECATED. This flag never did anything anyway."), + ('watch', 'w', "Watch file system and rebuild on change (defaults to off)"), ]) @timed def webpack(options): """ Run a Webpack build. - - This is a DEPRECATED COMPATIBILITY WRAPPER. Use `npm run webpack` instead. """ settings = getattr(options, 'settings', Env.DEVSTACK_SETTINGS) result = Env.get_django_settings(['STATIC_ROOT', 'WEBPACK_CONFIG_PATH'], "lms", settings=settings) @@ -330,20 +318,44 @@ def webpack(options): static_root_cms, = Env.get_django_settings(["STATIC_ROOT"], "cms", settings=settings) js_env_extra_config_setting, = Env.get_django_json_settings(["JS_ENV_EXTRA_CONFIG"], "cms", settings=settings) js_env_extra_config = json.dumps(js_env_extra_config_setting or "{}") - node_env = "development" if config_path == 'webpack.dev.config.js' else "production" - run_deprecated_command_wrapper( - old_command="paver webpack", - ignored_old_flags=(set(["watch"]) & set(options)), - new_command=' '.join([ - f"WEBPACK_CONFIG_PATH={config_path}", - f"NODE_ENV={node_env}", - f"STATIC_ROOT_LMS={static_root_lms}", - f"STATIC_ROOT_CMS={static_root_cms}", - f"JS_ENV_EXTRA_CONFIG={js_env_extra_config}", - "npm", - "run", - "webpack", - ]), + environment = ( + "NODE_ENV={node_env} STATIC_ROOT_LMS={static_root_lms} STATIC_ROOT_CMS={static_root_cms} " + "JS_ENV_EXTRA_CONFIG={js_env_extra_config}" + ).format( + node_env="development" if config_path == 'webpack.dev.config.js' else "production", + static_root_lms=static_root_lms, + static_root_cms=static_root_cms, + js_env_extra_config=js_env_extra_config, + ) + sh( + cmd( + '{environment} webpack --config={config_path}'.format( + environment=environment, + config_path=config_path + ) + ) + ) + + +def execute_webpack_watch(settings=None): + """ + Run the Webpack file system watcher. + """ + # We only want Webpack to re-run on changes to its own entry points, + # not all JS files, so we use its own watcher instead of subclassing + # from Watchdog like the other watchers do. + + result = Env.get_django_settings(["STATIC_ROOT", "WEBPACK_CONFIG_PATH"], "lms", settings=settings) + static_root_lms, config_path = result + static_root_cms, = Env.get_django_settings(["STATIC_ROOT"], "cms", settings=settings) + run_background_process( + 'STATIC_ROOT_LMS={static_root_lms} STATIC_ROOT_CMS={static_root_cms} webpack {options}'.format( + options='--watch --config={config_path}'.format( + config_path=config_path + ), + static_root_lms=static_root_lms, + static_root_cms=static_root_cms, + ) ) @@ -399,19 +411,39 @@ def watch_assets(options): return theme_dirs = ':'.join(get_parsed_option(options, 'theme_dirs', [])) - run_deprecated_command_wrapper( - old_command="paver watch_assets", - ignored_old_flags=(set(["debug", "themes", "settings", "background"]) & set(options)), - new_command=shlex.join([ + command = shlex.join( + [ *( - ["env", f"COMPREHENSIVE_THEME_DIRS={theme_dirs}"] - if theme_dirs else [] + ["env", f"EDX_PLATFORM_THEME_DIRS={theme_dirs}"] if theme_dirs else [] ), "npm", "run", "watch", - ]), + ] ) + depr_warning = ( + "\n" + + "⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ \n" + + "\n" + + "WARNING: 'paver watch_assets' is DEPRECATED! It will be removed before Sumac.\n" + + "The command you ran is now just a temporary wrapper around a new,\n" + + "supported command, which you should use instead:\n" + + "\n" + + f"\t{command}\n" + + "\n" + + "Details: https://github.com/openedx/edx-platform/issues/31895\n" + + "\n" + + ("WARNING: ignoring deprecated flag '--debug'\n" if options.get("debug") else "") + + ("WARNING: ignoring deprecated flag '--themes'\n" if options.get("themes") else "") + + ("WARNING: ignoring deprecated flag '--settings'\n" if options.get("settings") else "") + + ("WARNING: ignoring deprecated flag '--background'\n" if options.get("background") else "") + + "⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ \n" + + "\n" + ) + # Print deprecation warning twice so that it's more likely to be seen in the logs. + print(depr_warning) + sh(command) + print(depr_warning) @task @@ -424,19 +456,10 @@ def watch_assets(options): def update_assets(args): """ Compile Sass, then collect static assets. - - This is a DEPRECATED COMPATIBILITY WRAPPER around other DEPRECATED COMPATIBILITY WRAPPERS. - The aggregate affect of this command can be achieved with this sequence of commands instead: - - * pip install -r requirements/edx/assets.txt # replaces install_python_prereqs - * npm clean-install # replaces install_node_prereqs - * npm run build # replaces execute_compile_sass and webpack - * ./manage.py lms collectstatic --noinput # replaces collect_assets (for LMS) - * ./manage.py cms collectstatic --noinput # replaces collect_assets (for CMS) """ parser = argparse.ArgumentParser(prog='paver update_assets') parser.add_argument( - 'system', type=str, nargs='*', default=["lms", "studio"], + 'system', type=str, nargs='*', default=ALL_SYSTEMS, help="lms or studio", ) parser.add_argument( @@ -465,17 +488,18 @@ def update_assets(args): ) parser.add_argument( '--themes', type=str, nargs='+', default=None, - help="list of themes to compile sass for. ignored when --watch is used; all themes are watched.", + help="list of themes to compile sass for", ) parser.add_argument( - '--collect-log', dest="collect_log_dir", default=None, + '--collect-log', dest=COLLECTSTATIC_LOG_DIR_ARG, default=None, help="When running collectstatic, direct output to specified log directory", ) parser.add_argument( '--wait', type=float, default=0.0, - help="DEPRECATED. Watchdog's default wait time is now used.", + help="How long to pause between filesystem scans" ) args = parser.parse_args(args) + collect_log_args = {} # Build Webpack call_task('pavelib.assets.webpack', options={'settings': args.settings}) @@ -484,12 +508,11 @@ def update_assets(args): execute_compile_sass(args) if args.collect: + if args.debug or args.debug_collect: + collect_log_args.update({COLLECTSTATIC_LOG_DIR_ARG: None}) + if args.collect_log_dir: - collect_log_args = {"collect_log_dir": args.collect_log_dir} - elif args.debug or args.debug_collect: - collect_log_args = {"collect_log_dir": None} - else: - collect_log_args = {} + collect_log_args.update({COLLECTSTATIC_LOG_DIR_ARG: args.collect_log_dir}) collect_assets(args.system, args.settings, **collect_log_args) diff --git a/pavelib/paver_tests/test_assets.py b/pavelib/paver_tests/test_assets.py index 3578a5043f7e..029a8db67c4a 100644 --- a/pavelib/paver_tests/test_assets.py +++ b/pavelib/paver_tests/test_assets.py @@ -1,130 +1,305 @@ """Unit tests for the Paver asset tasks.""" -import json + import os -from pathlib import Path from unittest import TestCase from unittest.mock import patch import ddt -import paver.easy -from paver import tasks +import paver.tasks +from paver.easy import call_task, path -import pavelib.assets -from pavelib.assets import Env +from pavelib.assets import COLLECTSTATIC_LOG_DIR_ARG, collect_assets +from ..utils.envs import Env +from .utils import PaverTestCase -REPO_ROOT = Path(__file__).parent.parent.parent +ROOT_PATH = path(os.path.dirname(os.path.dirname(os.path.dirname(__file__)))) +TEST_THEME_DIR = ROOT_PATH / "common/test/test-theme" -LMS_SETTINGS = { - "WEBPACK_CONFIG_PATH": "webpack.fake.config.js", - "STATIC_ROOT": "/fake/lms/staticfiles", -} -CMS_SETTINGS = { - "WEBPACK_CONFIG_PATH": "webpack.fake.config", - "STATIC_ROOT": "/fake/cms/staticfiles", - "JS_ENV_EXTRA_CONFIG": json.dumps({"key1": [True, False], "key2": {"key2.1": 1369, "key2.2": "1369"}}), -} +@ddt.ddt +class TestPaverAssetTasks(PaverTestCase): + """ + Test the Paver asset tasks. + """ + @ddt.data( + [""], + ["--force"], + ["--debug"], + ["--system=lms"], + ["--system=lms --force"], + ["--system=studio"], + ["--system=studio --force"], + ["--system=lms,studio"], + ["--system=lms,studio --force"], + ) + @ddt.unpack + def test_compile_sass(self, options): + """ + Test the "compile_sass" task. + """ + parameters = options.split(" ") + system = [] + if '--system=studio' not in parameters: + system += ['lms'] + if '--system=lms' not in parameters: + system += ['studio'] + debug = '--debug' in parameters + force = '--force' in parameters + self.reset_task_messages() + call_task('pavelib.assets.compile_sass', options={'system': system, 'debug': debug, 'force': force}) + expected_messages = [] + if force: + expected_messages.append('rm -rf common/static/css/*.css') + expected_messages.append('libsass common/static/sass') + if "lms" in system: + if force: + expected_messages.append('rm -rf lms/static/css/*.css') + expected_messages.append('libsass lms/static/sass') + expected_messages.append( + 'rtlcss lms/static/css/bootstrap/lms-main.css lms/static/css/bootstrap/lms-main-rtl.css' + ) + expected_messages.append( + 'rtlcss lms/static/css/discussion/lms-discussion-bootstrap.css' + ' lms/static/css/discussion/lms-discussion-bootstrap-rtl.css' + ) + if force: + expected_messages.append('rm -rf lms/static/certificates/css/*.css') + expected_messages.append('libsass lms/static/certificates/sass') + if "studio" in system: + if force: + expected_messages.append('rm -rf cms/static/css/*.css') + expected_messages.append('libsass cms/static/sass') + expected_messages.append( + 'rtlcss cms/static/css/bootstrap/studio-main.css cms/static/css/bootstrap/studio-main-rtl.css' + ) -def _mock_get_django_settings(django_settings, system, settings=None): # pylint: disable=unused-argument - return [(LMS_SETTINGS if system == "lms" else CMS_SETTINGS)[s] for s in django_settings] + assert len(self.task_messages) == len(expected_messages) @ddt.ddt -@patch.object(Env, 'get_django_settings', _mock_get_django_settings) -@patch.object(Env, 'get_django_json_settings', _mock_get_django_settings) -class TestDeprecatedPaverAssets(TestCase): +class TestPaverThemeAssetTasks(PaverTestCase): """ - Simple test to ensure that the soon-to-be-removed Paver commands are correctly translated into the new npm-run - commands. + Test the Paver asset tasks. """ - def setUp(self): - super().setUp() - self.maxDiff = None - os.environ['NO_PREREQ_INSTALL'] = 'true' - tasks.environment = tasks.Environment() + @ddt.data( + [""], + ["--force"], + ["--debug"], + ["--system=lms"], + ["--system=lms --force"], + ["--system=studio"], + ["--system=studio --force"], + ["--system=lms,studio"], + ["--system=lms,studio --force"], + ) + @ddt.unpack + def test_compile_theme_sass(self, options): + """ + Test the "compile_sass" task. + """ + parameters = options.split(" ") + system = [] + + if '--system=studio' not in parameters: + system += ['lms'] + if "--system=lms" not in parameters: + system += ['studio'] + debug = '--debug' in parameters + force = '--force' in parameters + + self.reset_task_messages() + call_task( + 'pavelib.assets.compile_sass', + options=dict( + system=system, + debug=debug, + force=force, + theme_dirs=[TEST_THEME_DIR.dirname()], + themes=[TEST_THEME_DIR.basename()] + ), + ) + expected_messages = [] + if force: + expected_messages.append('rm -rf common/static/css/*.css') + expected_messages.append('libsass common/static/sass') + + if 'lms' in system: + expected_messages.append('mkdir_p ' + repr(TEST_THEME_DIR / 'lms/static/css')) + if force: + expected_messages.append( + f'rm -rf {str(TEST_THEME_DIR)}/lms/static/css/*.css' + ) + expected_messages.append("libsass lms/static/sass") + expected_messages.append( + 'rtlcss {test_theme_dir}/lms/static/css/bootstrap/lms-main.css' + ' {test_theme_dir}/lms/static/css/bootstrap/lms-main-rtl.css'.format( + test_theme_dir=str(TEST_THEME_DIR), + ) + ) + expected_messages.append( + 'rtlcss {test_theme_dir}/lms/static/css/discussion/lms-discussion-bootstrap.css' + ' {test_theme_dir}/lms/static/css/discussion/lms-discussion-bootstrap-rtl.css'.format( + test_theme_dir=str(TEST_THEME_DIR), + ) + ) + if force: + expected_messages.append( + f'rm -rf {str(TEST_THEME_DIR)}/lms/static/css/*.css' + ) + expected_messages.append( + f'libsass {str(TEST_THEME_DIR)}/lms/static/sass' + ) + if force: + expected_messages.append('rm -rf lms/static/css/*.css') + expected_messages.append('libsass lms/static/sass') + expected_messages.append( + 'rtlcss lms/static/css/bootstrap/lms-main.css lms/static/css/bootstrap/lms-main-rtl.css' + ) + expected_messages.append( + 'rtlcss lms/static/css/discussion/lms-discussion-bootstrap.css' + ' lms/static/css/discussion/lms-discussion-bootstrap-rtl.css' + ) + if force: + expected_messages.append('rm -rf lms/static/certificates/css/*.css') + expected_messages.append('libsass lms/static/certificates/sass') - def tearDown(self): - super().tearDown() - del os.environ['NO_PREREQ_INSTALL'] + if "studio" in system: + expected_messages.append('mkdir_p ' + repr(TEST_THEME_DIR / 'cms/static/css')) + if force: + expected_messages.append( + f'rm -rf {str(TEST_THEME_DIR)}/cms/static/css/*.css' + ) + expected_messages.append('libsass cms/static/sass') + expected_messages.append( + 'rtlcss {test_theme_dir}/cms/static/css/bootstrap/studio-main.css' + ' {test_theme_dir}/cms/static/css/bootstrap/studio-main-rtl.css'.format( + test_theme_dir=str(TEST_THEME_DIR), + ) + ) + if force: + expected_messages.append( + f'rm -rf {str(TEST_THEME_DIR)}/cms/static/css/*.css' + ) + expected_messages.append( + f'libsass {str(TEST_THEME_DIR)}/cms/static/sass' + ) + if force: + expected_messages.append('rm -rf cms/static/css/*.css') + expected_messages.append('libsass cms/static/sass') + expected_messages.append( + 'rtlcss cms/static/css/bootstrap/studio-main.css cms/static/css/bootstrap/studio-main-rtl.css' + ) + + assert len(self.task_messages) == len(expected_messages) + + +@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"]) + if specified_log_loc is None: + collect_assets( + systems, + Env.DEVSTACK_SETTINGS + ) + else: + collect_assets( + systems, + Env.DEVSTACK_SETTINGS, + **specified_log_dict + ) + self._assert_correct_messages(log_location=log_loc, systems=systems) + + 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} + collect_assets(systems, Env.DEVSTACK_SETTINGS, **kwargs) + self._assert_correct_messages(log_location=expected_log_loc, systems=systems) + + def _assert_correct_messages(self, log_location, systems): + """ + Asserts that the expected commands were run. + + We just extract the pieces we care about here instead of specifying an + exact command, so that small arg changes don't break this test. + """ + for i, sys in enumerate(systems): + msg = self.task_messages[i] + assert msg.startswith(f'python manage.py {sys}') + assert ' collectstatic ' in msg + assert f'--settings={Env.DEVSTACK_SETTINGS}' in msg + assert msg.endswith(f' {log_location}') + + +@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( - dict( - task_name='pavelib.assets.compile_sass', - args=[], - kwargs={}, - expected=["npm run compile-sass --"], - ), - dict( - task_name='pavelib.assets.compile_sass', - args=[], - kwargs={"system": "lms,studio"}, - expected=["npm run compile-sass --"], - ), - dict( - task_name='pavelib.assets.compile_sass', - args=[], - kwargs={"debug": True}, - expected=["npm run compile-sass-dev --"], - ), - dict( - task_name='pavelib.assets.compile_sass', - args=[], - kwargs={"system": "lms"}, - expected=["npm run compile-sass -- --skip-cms"], - ), - dict( - task_name='pavelib.assets.compile_sass', - args=[], - kwargs={"system": "studio"}, - expected=["npm run compile-sass -- --skip-lms"], - ), - dict( - task_name='pavelib.assets.compile_sass', - args=[], - kwargs={"system": "cms", "theme_dirs": f"{REPO_ROOT}/common/test,{REPO_ROOT}/themes"}, - expected=[ - "npm run compile-sass -- --skip-lms " + - f"--theme-dir {REPO_ROOT}/common/test --theme-dir {REPO_ROOT}/themes" - ], - ), - dict( - task_name='pavelib.assets.compile_sass', - args=[], - kwargs={"theme_dirs": f"{REPO_ROOT}/common/test,{REPO_ROOT}/themes", "themes": "red-theme,test-theme"}, - expected=[ - "npm run compile-sass -- " + - f"--theme-dir {REPO_ROOT}/common/test --theme-dir {REPO_ROOT}/themes " + - "--theme red-theme --theme test-theme" - ], - ), - dict( - task_name='pavelib.assets.update_assets', - args=["lms", "studio", "--settings=fake.settings"], - kwargs={}, - expected=[ - ( - "WEBPACK_CONFIG_PATH=webpack.fake.config.js " + - "NODE_ENV=production " + - "STATIC_ROOT_LMS=/fake/lms/staticfiles " + - "STATIC_ROOT_CMS=/fake/cms/staticfiles " + - 'JS_ENV_EXTRA_CONFIG=' + + - '"{\\"key1\\": [true, false], \\"key2\\": {\\"key2.1\\": 1369, \\"key2.2\\": \\"1369\\"}}" ' + - "npm run webpack" - ), - "python manage.py lms --settings=fake.settings compile_sass lms ", - "python manage.py cms --settings=fake.settings compile_sass cms ", - ( - "( ./manage.py lms --settings=fake.settings collectstatic --noinput ) && " + - "( ./manage.py cms --settings=fake.settings collectstatic --noinput )" - ), - ], - ), + [{"expected_substring": "> /dev/null"}], # go to /dev/null by default + [{"cmd_args": ["--debug"], "expected_substring": "collectstatic"}] # TODO: make this regex ) @ddt.unpack - @patch.object(pavelib.assets, 'sh') - def test_paver_assets_wrapper_invokes_new_commands(self, mock_sh, task_name, args, kwargs, expected): - paver.easy.call_task(task_name, args=args, options=kwargs) - assert [call_args[0] for (call_args, call_kwargs) in mock_sh.call_args_list] == expected + 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=f"{expected_substring} not found in messages" + ) + + 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/scripts/compile_sass.py b/scripts/compile_sass.py index ec1efee24d2b..41a2d56b3bda 100755 --- a/scripts/compile_sass.py +++ b/scripts/compile_sass.py @@ -67,12 +67,14 @@ "theme_dirs", metavar="PATH", multiple=True, - envvar="COMPREHENSIVE_THEME_DIRS", - type=click.Path(path_type=Path), + envvar="EDX_PLATFORM_THEME_DIRS", + type=click.Path( + exists=True, file_okay=False, readable=True, writable=True, path_type=Path + ), help=( "Consider sub-dirs of PATH as themes. " "Multiple theme dirs are accepted. " - "If none are provided, we look at colon-separated paths on the COMPREHENSIVE_THEME_DIRS env var." + "If none are provided, we look at colon-separated paths on the EDX_PLATFORM_THEME_DIRS env var." ), ) @click.option( diff --git a/scripts/watch_sass.sh b/scripts/watch_sass.sh index e88cb9e3cd6f..68d4b1f471a5 100755 --- a/scripts/watch_sass.sh +++ b/scripts/watch_sass.sh @@ -4,11 +4,11 @@ # Invoke from repo root as `npm run watch-sass`. # By default, only watches default Sass. -# To watch themes too, provide colon-separated paths in the COMPREHENSIVE_THEME_DIRS environment variable. +# To watch themes too, provide colon-separated paths in the EDX_PLATFORM_THEME_DIRS environment variable. # Each path will be treated as a "theme dir", which means that every immediate child directory is watchable as a theme. # For example: # -# COMPREHENSIVE_THEME_DIRS=/openedx/themes:./themes npm run watch-sass +# EDX_PLATFORM_THEME_DIRS=/openedx/themes:./themes npm run watch-sass # # would watch default Sass as well as /openedx/themes/indigo, /openedx/themes/mytheme, ./themes/red-theme, etc.