From cbc6b0701693107cdea9ddebbbf87aa74bb0323d Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Tue, 4 Apr 2017 11:12:37 +0200 Subject: [PATCH 1/4] Fix up some legacy code, removing PROJECT_PATH and invalid paths in 'setup' management command --- .../management/commands/screenshots.py | 8 +++++--- kalite/distributed/management/commands/setup.py | 17 ++++++----------- kalite/distributed/tests/code_tests.py | 1 - kalite/settings/base.py | 6 ------ 4 files changed, 11 insertions(+), 21 deletions(-) diff --git a/kalite/distributed/management/commands/screenshots.py b/kalite/distributed/management/commands/screenshots.py index b3c38d2569..8354b20904 100644 --- a/kalite/distributed/management/commands/screenshots.py +++ b/kalite/distributed/management/commands/screenshots.py @@ -52,7 +52,7 @@ class Command(BaseCommand): action='store', dest='output_dir', default=None, - help='Specify the output directory relative to the project base directory.'), + help='Specify the output directory relative to current working directory.'), make_option('--no-del', action='store_true', dest='no_del', @@ -184,8 +184,10 @@ def __init__(self, *args, **kwargs): # make sure output path exists and is empty if kwargs['output_dir']: - self.output_path = os.path.join( os.path.realpath(os.path.join(settings.PROJECT_PATH, '..')), - kwargs['output_dir']) + self.output_path = os.path.join( + os.path.realpath(os.getcwd()), + kwargs['output_dir'] + ) else: self.output_path = settings.SCREENSHOTS_OUTPUT_PATH ensure_dir(self.output_path) diff --git a/kalite/distributed/management/commands/setup.py b/kalite/distributed/management/commands/setup.py index d584be45eb..bb4da960b7 100644 --- a/kalite/distributed/management/commands/setup.py +++ b/kalite/distributed/management/commands/setup.py @@ -29,7 +29,7 @@ from django.core.management import call_command from django.core.management.base import BaseCommand, CommandError -from kalite import ROOT_DATA_PATH +import kalite from kalite.facility.models import Facility from kalite.version import VERSION, SHORTVERSION from kalite.i18n.base import CONTENT_PACK_URL_TEMPLATE, reset_content_db @@ -42,7 +42,7 @@ CONTENTPACK_URL = CONTENT_PACK_URL_TEMPLATE.format( version=SHORTVERSION, langcode="en", suffix="") -PRESEED_DIR = os.path.join(ROOT_DATA_PATH, "preseed") +PRESEED_DIR = os.path.join(kalite.ROOT_DATA_PATH, "preseed") # Examples: # contentpack.en.zip @@ -477,15 +477,10 @@ def handle(self, *args, **options): if not settings.CENTRAL_SERVER: kalite_executable = 'kalite' - if not spawn.find_executable('kalite'): - if os.name == 'posix': - start_script_path = os.path.realpath( - os.path.join(settings.PROJECT_PATH, "..", "bin", kalite_executable)) - else: - start_script_path = os.path.realpath( - os.path.join(settings.PROJECT_PATH, "..", "bin", "windows", "kalite.bat")) - else: + if spawn.find_executable(kalite_executable): start_script_path = kalite_executable + else: + start_script_path = None # Run annotate_content_items, on the distributed server. print("Annotating availability of all content, checking for content in this directory: (%s)" % @@ -501,7 +496,7 @@ def handle(self, *args, **options): print( "You can now start KA Lite with the following command:\n\n\t%s start\n\n" % start_script_path) - if options['interactive']: + if options['interactive'] and start_script_path: if raw_input_yn("Do you wish to start the server now?"): print("Running {0} start".format(start_script_path)) p = subprocess.Popen( diff --git a/kalite/distributed/tests/code_tests.py b/kalite/distributed/tests/code_tests.py index baf3cb7fa4..8c63b43ee6 100644 --- a/kalite/distributed/tests/code_tests.py +++ b/kalite/distributed/tests/code_tests.py @@ -50,7 +50,6 @@ def compute_app_dependencies(cls): global_vars = copy.copy(globals()) global_vars.update({ "__file__": settings_filepath, # must let the app's settings file be set to that file! - 'PROJECT_PATH': settings.PROJECT_PATH, 'ROOT_DATA_PATH': getattr(settings, 'ROOT_DATA_PATH', os.path.join(settings.PROJECT_PATH, 'data')), }) app_settings = {'__package__': app} # explicit setting of the __package__, to allow absolute package ref'ing diff --git a/kalite/settings/base.py b/kalite/settings/base.py index 856183efcb..8158f1207a 100644 --- a/kalite/settings/base.py +++ b/kalite/settings/base.py @@ -119,12 +119,6 @@ _data_path = ROOT_DATA_PATH -# BEING DEPRECATED, PLEASE DO NOT USE PROJECT_PATH! -PROJECT_PATH = os.environ.get( - "KALITE_HOME", - os.path.join(os.path.expanduser("~"), ".kalite") -) - ################################################### # CHANNEL and CONTENT DATA From 9177bde4396f46592db0045b24729bab40e488b3 Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Tue, 4 Apr 2017 11:17:06 +0200 Subject: [PATCH 2/4] Release note for #4104 --- docs/installguide/release_notes.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/installguide/release_notes.rst b/docs/installguide/release_notes.rst index afb64bf6d2..d00ceba062 100644 --- a/docs/installguide/release_notes.rst +++ b/docs/installguide/release_notes.rst @@ -24,9 +24,16 @@ Known issues * **Chrome 55-56** has issues scrolling the menus on touch devices. Upgrading to Chrome 57 fixes this. :url-issue:`5407` * **Windows** needs at least Python 2.7.11. The Windows installer for KA Lite will install the latest version of Python. If you installed KA Lite in another way, and your Python installation is more than a year old, you probably have to upgrade Python - you can fetch the latest 2.7.12 version `here `__. * **Windows** installer tray application option "Run on start" does not work, see `learningequality/installers#106 `__ (also contains `a work-around`__) + * **Windows + IE9** One-Click device registration is broken. Work-around: Use a different browser or use manual device registration. :url-issue:`5409` * **Firefox 47**: Subtitles are misaligned in the video player. This is fixed by upgrading Firefox. +Code cleanup +^^^^^^^^^^^^ + + * Remove ``PROJECT_PATH`` from ``kalite.settings.base`` (it wasn't a configurable setting). :url-issue:`4104` + + 0.17.0 ------ From a2576ac49c33563ede88e68b88bb525f80c42951 Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Tue, 4 Apr 2017 12:29:14 +0200 Subject: [PATCH 3/4] Make tests compatible with Selenium 3.3+ and geckodriver 0.15 --- docs/installguide/release_notes.rst | 2 +- kalite/testing/testrunner.py | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/installguide/release_notes.rst b/docs/installguide/release_notes.rst index d00ceba062..b41e6c15d3 100644 --- a/docs/installguide/release_notes.rst +++ b/docs/installguide/release_notes.rst @@ -32,7 +32,7 @@ Code cleanup ^^^^^^^^^^^^ * Remove ``PROJECT_PATH`` from ``kalite.settings.base`` (it wasn't a configurable setting). :url-issue:`4104` - + * Make tests run on Selenium 3.3+ and geckodriver 0.15 (Firefox) :url-issue:`5429` 0.17.0 ------ diff --git a/kalite/testing/testrunner.py b/kalite/testing/testrunner.py index 1d910ae03f..700aef9afe 100644 --- a/kalite/testing/testrunner.py +++ b/kalite/testing/testrunner.py @@ -87,7 +87,7 @@ def get_options(): option_info = {"--behave_browser": True} for fixed, keywords in options: - # Look for the long version of this option + # Look for the long of this option long_option = None for option in fixed: if option.startswith("--"): @@ -170,7 +170,12 @@ def build_suite(self, test_labels, extra_tests, **kwargs): # Output Firefox version, needed to understand Selenium compatibility # issues browser = webdriver.Firefox() - logging.info("Successfully setup Firefox {0}".format(browser.capabilities['version'])) + browser_version = getattr( + browser.capabilities, + 'browserVersion', # Selenium 3+ + browser.capabilities.get('version', None) # Selenium 2 + ) + logging.info("Successfully setup Firefox {0}".format(browser_version)) browser.quit() if not database_exists() or os.path.getsize(database_path()) < 1024 * 1024: From 97b4a18a32a5bc0aaa6a20bc0b741f6a1a21219e Mon Sep 17 00:00:00 2001 From: Benjamin Bach Date: Tue, 4 Apr 2017 12:32:42 +0200 Subject: [PATCH 4/4] Remove unused 'code_tests', unconventional test pattern (it doesn't contain any test_* methods, thus nothing ever gets called) --- kalite/distributed/tests/__init__.py | 1 - kalite/distributed/tests/code_tests.py | 171 ------------------------- 2 files changed, 172 deletions(-) delete mode 100644 kalite/distributed/tests/code_tests.py diff --git a/kalite/distributed/tests/__init__.py b/kalite/distributed/tests/__init__.py index 61c4deb9db..b83ccfc563 100644 --- a/kalite/distributed/tests/__init__.py +++ b/kalite/distributed/tests/__init__.py @@ -1,3 +1,2 @@ -from code_tests import * from url_tests import * from browser_tests import * diff --git a/kalite/distributed/tests/code_tests.py b/kalite/distributed/tests/code_tests.py deleted file mode 100644 index 8c63b43ee6..0000000000 --- a/kalite/distributed/tests/code_tests.py +++ /dev/null @@ -1,171 +0,0 @@ -import copy -import glob -import importlib -import os -import re - -from django.conf import settings; logging = settings.LOG -from django.utils import unittest - - -def get_module_files(module_dirpath, file_filter_fn): - source_files = [] - for root, dirs, files in os.walk(module_dirpath): # Recurse over all files - source_files += [os.path.join(root, f) for f in files if file_filter_fn(f)] # Filter py files - return source_files - - -class KALiteCodeTest(unittest.TestCase): - testable_packages = ['kalite', 'securesync', 'fle_utils.config', 'fle_utils.chronograph', 'fle_utils.deployments', 'fle_utils.feeds'] - - def __init__(self, *args, **kwargs): - """ """ - super(KALiteCodeTest, self).__init__(*args, **kwargs) - if not hasattr(self.__class__, 'our_apps'): - self.__class__.our_apps = set([app for app in settings.INSTALLED_APPS if app in self.testable_packages or app.split('.')[0] in self.testable_packages]) - self.__class__.compute_app_dependencies() - self.__class__.compute_app_urlpatterns() - - @classmethod - def compute_app_dependencies(cls): - """For each app in settings.INSTALLED_APPS, load that app's settings.py to grab its dependencies - from its own INSTALLED_APPS. - - Note: assumes cls.our_apps has already been computed. - """ - cls.our_app_dependencies = {} - - # Get each app's dependencies. - for app in cls.our_apps: - module = importlib.import_module(app) - module_dirpath = os.path.dirname(module.__file__) - settings_filepath = os.path.join(module_dirpath, 'settings.py') - - if not os.path.exists(settings_filepath): - our_app_dependencies = [] - else: - # Load the settings.py file. This requires settings some (expected) global variables, - # such as PROJECT_PATH and ROOT_DATA_PATH, such that the scripts execute stand-alone - # TODO: make these scripts execute stand-alone. - global_vars = copy.copy(globals()) - global_vars.update({ - "__file__": settings_filepath, # must let the app's settings file be set to that file! - 'ROOT_DATA_PATH': getattr(settings, 'ROOT_DATA_PATH', os.path.join(settings.PROJECT_PATH, 'data')), - }) - app_settings = {'__package__': app} # explicit setting of the __package__, to allow absolute package ref'ing - execfile(settings_filepath, global_vars, app_settings) - our_app_dependencies = [anapp for anapp in app_settings.get('INSTALLED_APPS', []) if anapp in cls.our_apps] - - cls.our_app_dependencies[app] = our_app_dependencies - - @classmethod - def get_fle_imports(cls, app): - """Recurses over files within an app, searches each file for KA Lite-relevant imports, - then grabs the fully-qualified module import for each import on each line. - - The logic is hacky and makes assumptions (no multi-line imports, but handles comma-delimited import lists), - but generally works. - - Returns a dict of tuples - key: filepath - value: (actual code line, reconstructed import) - """ - module = importlib.import_module(app) - module_dirpath = os.path.dirname(module.__file__) - - imports = {} - - py_files = get_module_files(module_dirpath, lambda f: os.path.splitext(f)[-1] in ['.py']) - for filepath in py_files: - lines = open(filepath, 'r').readlines() # Read the entire file - import_lines = [l.strip() for l in lines if 'import' in l] # Grab lines containing 'import' - our_import_lines = [] - for import_line in import_lines: - for rexp in [r'^\s*from\s+(.*)\s+import\s+(.*)\s*$', r'^\s*import\s+(.*)\s*$']: # Match 'import X' and 'from A import B' syntaxes - matches = re.match(rexp, import_line) - groups = matches and list(matches.groups()) or [] - import_mod = [] - for list_item in ((groups and groups[-1].split(",")) or []): # Takes the last item (which get split into a CSV list) - cur_item = '.'.join([item.strip() for item in (groups[0:-1] + [list_item])]) # Reconstitute to fully-qualified import - if any([a for a in cls.our_apps if a in cur_item]): # Search for the app in all the apps we know matter - our_import_lines.append((import_line, cur_item)) # Store line and import item as a tuple - if app in cur_item: # Special case: warn if fully qualified import within an app (should be relative) - logging.warn("*** Please use relative imports within an app (%s: found '%s')" % (app, import_line)) - else: # Not a relevant / tracked import - logging.debug("*** Skipping import: %s (%s)" % (import_line, cur_item)) - imports[filepath] = our_import_lines - return imports - - @classmethod - def compute_app_urlpatterns(cls): - """For each app in settings.INSTALLED_APPS, load that app's *urls.py to grab its - defined URLS. - - Note: assumes cls.our_apps has already been computed. - """ - cls.app_urlpatterns = {} - - # Get each app's dependencies. - for app in cls.our_apps: - module = importlib.import_module(app) - module_dirpath = os.path.dirname(module.__file__) - settings_filepath = os.path.join(module_dirpath, 'settings.py') - - urlpatterns = [] - source_files = get_module_files(module_dirpath, lambda f: 'urls' in f and os.path.splitext(f)[-1] in ['.py']) - for filepath in source_files: - fq_urlconf_module = app + os.path.splitext(filepath[len(module_dirpath):])[0].replace('/', '.') - - logging.info('Processing urls file: %s' % fq_urlconf_module) - mod = importlib.import_module(fq_urlconf_module) - urlpatterns += mod.urlpatterns - - cls.app_urlpatterns[app] = urlpatterns - - - @classmethod - def get_url_reversals(cls, app): - """Recurses over files within an app, searches each file for KA Lite-relevant URL confs, - then grabs the fully-qualified module import for each import on each line. - - The logic is hacky and makes assumptions (no multi-line imports, but handles comma-delimited import lists), - but generally works. - - Returns a dict of tuples - key: filepath - value: (actual code line, reconstructed import) - """ - - module = importlib.import_module(app) - module_dirpath = os.path.dirname(module.__file__) - - url_reversals = {} - - source_files = get_module_files(module_dirpath, lambda f: os.path.splitext(f)[-1] in ['.py', '.html']) - for filepath in source_files: - mod_revs = [] - for line in open(filepath, 'r').readlines(): - new_revs = [] - for rexp in [r""".*reverse\(\s*['"]([^\)\s,]+)['"].*""", r""".*\{%\s*url\s+['"]([^%\s]+)['"].*"""]: # Match 'reverse(URI)' and '{% url URI %}' syntaxes - - matches = re.match(rexp, line) - groups = matches and list(matches.groups()) or [] - if groups: - new_revs += groups - logging.debug('Found: %s; %s' % (filepath, line)) - - if not new_revs and ('reverse(' in line or '{% url' in line): - logging.debug("\tSkip: %s; %s" % (filepath, line)) - mod_revs += new_revs - - url_reversals[filepath] = mod_revs - return url_reversals - - - @classmethod - def get_url_modules(cls, url_name): - """Given a URL name, returns all INSTALLED_APPS that have that URL name defined within the app.""" - - # Search patterns across all known apps that are named have that name. - found_modules = [app for app, pats in cls.app_urlpatterns.iteritems() for pat in pats if getattr(pat, "name", None) == url_name] - return found_modules