From d8defaa45af1dd83f89917ff228e0267bff86e08 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 27 Jul 2016 14:38:01 -0400 Subject: [PATCH] Clarify how options are passed to paver tasks, so that pa11y subtasks get needed information --- pavelib/bok_choy.py | 23 ++-- .../paver_tests/test_paver_bok_choy_cmds.py | 42 +++++-- pavelib/utils/test/bokchoy_options.py | 38 ++++-- pavelib/utils/test/bokchoy_utils.py | 9 +- pavelib/utils/test/suites/bokchoy_suite.py | 113 ++++++++++-------- 5 files changed, 140 insertions(+), 85 deletions(-) diff --git a/pavelib/bok_choy.py b/pavelib/bok_choy.py index 8559bfc139..54502223a6 100644 --- a/pavelib/bok_choy.py +++ b/pavelib/bok_choy.py @@ -4,7 +4,12 @@ http://bok-choy.readthedocs.org/en/latest/ """ from paver.easy import task, needs, cmdopts, sh from pavelib.utils.test.suites.bokchoy_suite import BokChoyTestSuite, Pa11yCrawler -from pavelib.utils.test.bokchoy_options import BOKCHOY_OPTS +from pavelib.utils.test.bokchoy_options import ( + BOKCHOY_OPTS, + PA11Y_HTML, + PA11Y_COURSE_KEY, + PA11Y_FETCH_COURSE, +) from pavelib.utils.envs import Env from pavelib.utils.test.utils import check_firefox_version from pavelib.utils.passthrough_opts import PassthroughTask @@ -93,17 +98,11 @@ def perf_report_bokchoy(options, passthrough_options): run_bokchoy(options.perf_report_bokchoy, passthrough_options) -@needs('pavelib.prereqs.install_prereqs') -@cmdopts(BOKCHOY_OPTS + [ - ('with-html', 'w', 'Include html reports'), - make_option('--course-key', help='Course key for test course'), - make_option( - "--fetch-course", - action="store_true", - dest="should_fetch_course", - help='Course key for test course', - ), -]) +@needs('pavelib.prereqs.install_prereqs', 'get_test_course') +@cmdopts( + BOKCHOY_OPTS + [PA11Y_HTML, PA11Y_COURSE_KEY, PA11Y_FETCH_COURSE], + share_with=['get_test_course', 'prepare_bokchoy_run', 'load_courses'] +) @PassthroughTask @timed def pa11ycrawler(options, passthrough_options): diff --git a/pavelib/paver_tests/test_paver_bok_choy_cmds.py b/pavelib/paver_tests/test_paver_bok_choy_cmds.py index f2093932d2..37e3e5046c 100644 --- a/pavelib/paver_tests/test_paver_bok_choy_cmds.py +++ b/pavelib/paver_tests/test_paver_bok_choy_cmds.py @@ -5,10 +5,12 @@ Run just this test with: paver test_lib -t pavelib/paver_tests/test_paver_bok_ch import os import unittest +import ddt from mock import patch, call from test.test_support import EnvironmentVarGuard -from paver.easy import BuildFailure +from paver.easy import BuildFailure, call_task, environment from pavelib.utils.test.suites import BokChoyTestSuite, Pa11yCrawler +from pavelib.utils.test.suites.bokchoy_suite import DEMO_COURSE_TAR_GZ, DEMO_COURSE_IMPORT_DIR REPO_DIR = os.getcwd() @@ -173,6 +175,7 @@ class TestPaverBokChoyCmd(unittest.TestCase): suite.verbosity_processes_command +@ddt.ddt class TestPaverPa11yCrawlerCmd(unittest.TestCase): """ @@ -190,6 +193,9 @@ class TestPaverPa11yCrawlerCmd(unittest.TestCase): # Cleanup mocks self.addCleanup(mock_sh.stop) + # reset the options for all tasks + environment.options.clear() + def _expected_command(self, report_dir, start_urls): """ Returns the expected command to run pa11ycrawler. @@ -213,15 +219,31 @@ class TestPaverPa11yCrawlerCmd(unittest.TestCase): self._expected_command(suite.pa11y_report_dir, suite.start_urls) ) - def test_get_test_course(self): - suite = Pa11yCrawler('') - suite.get_test_course() - self._mock_sh.assert_has_calls([ - call( - 'wget {targz} -O {dir}demo_course.tar.gz'.format(targz=suite.tar_gz_file, dir=suite.imports_dir)), - call( - 'tar zxf {dir}demo_course.tar.gz -C {dir}'.format(dir=suite.imports_dir)), - ]) + @ddt.data( + (True, True, None), + (True, False, None), + (False, True, DEMO_COURSE_IMPORT_DIR), + (False, False, None), + ) + @ddt.unpack + def test_get_test_course(self, import_dir_set, should_fetch_course_set, downloaded_to): + options = {} + if import_dir_set: + options['imports_dir'] = 'some_import_dir' + if should_fetch_course_set: + options['should_fetch_course'] = True + + call_task('pavelib.utils.test.suites.bokchoy_suite.get_test_course', options=options) + + if downloaded_to is None: + self._mock_sh.assert_has_calls([]) + else: + self._mock_sh.assert_has_calls([ + call( + 'wget {targz} -O {dir}demo_course.tar.gz'.format(targz=DEMO_COURSE_TAR_GZ, dir=downloaded_to)), + call( + 'tar zxf {dir}demo_course.tar.gz -C {dir}'.format(dir=downloaded_to)), + ]) def test_generate_html_reports(self): suite = Pa11yCrawler('') diff --git a/pavelib/utils/test/bokchoy_options.py b/pavelib/utils/test/bokchoy_options.py index 84cdf2a639..cdb343ef16 100644 --- a/pavelib/utils/test/bokchoy_options.py +++ b/pavelib/utils/test/bokchoy_options.py @@ -8,19 +8,34 @@ import os from pavelib.utils.envs import Env +BOKCHOY_IMPORTS_DIR = ('imports-dir=', 'i', 'Directory containing (un-archived) courses to be imported') +BOKCHOY_IMPORTS_DIR_DEPR = ('imports_dir=', None, 'deprecated in favor of imports-dir') +BOKCHOY_DEFAULT_STORE = make_option( + "-s", "--default-store", + default=os.environ.get('DEFAULT_STORE', 'split'), + help='Default modulestore' +) +BOKCHOY_DEFAULT_STORE_DEPR = make_option( + "--default_store", + default=os.environ.get('DEFAULT_STORE', 'split'), + help='deprecated in favor of default-store' +) +BOKCHOY_FASTTEST = make_option('-a', '--fasttest', action='store_true', help='Skip some setup') +BOKCHOY_COVERAGERC = make_option('--coveragerc', help='coveragerc file to use during this test') + BOKCHOY_OPTS = [ ('test-spec=', 't', 'Specific test to run'), - make_option('-a', '--fasttest', action='store_true', help='Skip some setup'), + BOKCHOY_FASTTEST, ('skip-clean', 'C', 'Skip cleaning repository before running tests'), make_option('-r', '--serversonly', action='store_true', help='Prepare suite and leave servers running'), make_option('-o', '--testsonly', action='store_true', help='Assume servers are running and execute tests only'), - make_option("-s", "--default-store", default=os.environ.get('DEFAULT_STORE', 'split'), help='Default modulestore'), + BOKCHOY_DEFAULT_STORE, make_option( '-d', '--test-dir', default='tests', help='Directory for finding tests (relative to common/test/acceptance)' ), - ('imports-dir=', 'i', 'Directory containing (un-archived) courses to be imported'), + BOKCHOY_IMPORTS_DIR, make_option('-n', '--num-processes', type='int', help='Number of test threads (for multiprocessing)'), make_option( '-x', '--verify-xss', @@ -35,17 +50,13 @@ BOKCHOY_OPTS = [ make_option("--save-screenshots", action='store_true', dest="save_screenshots"), make_option("--report-dir", default=Env.BOK_CHOY_REPORT_DIR, help="Directory to store reports in"), - make_option( - "--default_store", - default=os.environ.get('DEFAULT_STORE', 'split'), - help='deprecated in favor of default-store' - ), + BOKCHOY_DEFAULT_STORE_DEPR, make_option( '-e', '--extra_args', default='', help='deprecated, pass extra options directly in the paver commandline' ), - ('imports_dir=', None, 'deprecated in favor of imports-dir'), + BOKCHOY_IMPORTS_DIR_DEPR, make_option('--num_processes', type='int', help='deprecated in favor of num-processes'), ('skip_clean', None, 'deprecated in favor of skip-clean'), make_option('--test_dir', default='tests', help='deprecated in favor of test-dir'), @@ -69,3 +80,12 @@ BOKCHOY_OPTS = [ help="deprecated in favor of save-screenshots" ), ] + +PA11Y_HTML = ('with-html', 'w', 'Include html reports') +PA11Y_COURSE_KEY = make_option('--course-key', help='Course key for test course') +PA11Y_FETCH_COURSE = make_option( + "--fetch-course", + action="store_true", + dest="should_fetch_course", + help='Course key for test course', +) diff --git a/pavelib/utils/test/bokchoy_utils.py b/pavelib/utils/test/bokchoy_utils.py index 8a4117e4fb..726e67b97e 100644 --- a/pavelib/utils/test/bokchoy_utils.py +++ b/pavelib/utils/test/bokchoy_utils.py @@ -6,10 +6,13 @@ import os import time import httplib import subprocess -from paver.easy import sh, task, cmdopts +from paver import tasks +from paver.easy import sh, task, cmdopts, needs 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.test.bokchoy_options import ( + BOKCHOY_COVERAGERC, BOKCHOY_DEFAULT_STORE, BOKCHOY_DEFAULT_STORE_DEPR +) from pavelib.utils.timer import timed try: @@ -21,7 +24,7 @@ __test__ = False # do not collect @task -@cmdopts(BOKCHOY_OPTS, share_with=['test_bokchoy', 'test_a11y', 'pa11ycrawler']) +@cmdopts([BOKCHOY_COVERAGERC, BOKCHOY_DEFAULT_STORE, BOKCHOY_DEFAULT_STORE_DEPR]) @timed def start_servers(options): """ diff --git a/pavelib/utils/test/suites/bokchoy_suite.py b/pavelib/utils/test/suites/bokchoy_suite.py index ef16eefbe1..d59d560c46 100644 --- a/pavelib/utils/test/suites/bokchoy_suite.py +++ b/pavelib/utils/test/suites/bokchoy_suite.py @@ -7,13 +7,18 @@ 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, cmdopts, task, needs, call_task +from paver.easy import sh, BuildFailure, cmdopts, task, needs, might_call, call_task from pavelib.utils.test.suites.suite import TestSuite from pavelib.utils.envs import Env 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.bokchoy_options import ( + BOKCHOY_IMPORTS_DIR, BOKCHOY_IMPORTS_DIR_DEPR, + BOKCHOY_DEFAULT_STORE, BOKCHOY_DEFAULT_STORE_DEPR, + BOKCHOY_FASTTEST, + PA11Y_FETCH_COURSE +) from pavelib.utils.test import utils as test_utils from pavelib.utils.timer import timed @@ -29,9 +34,12 @@ __test__ = False # do not collect DEFAULT_NUM_PROCESSES = 1 DEFAULT_VERBOSITY = 2 +DEMO_COURSE_TAR_GZ = "https://github.com/edx/demo-test-course/archive/master.tar.gz" +DEMO_COURSE_IMPORT_DIR = path('test_root/courses/') + @task -@cmdopts(BOKCHOY_OPTS, share_with=['test_bokchoy', 'test_a11y', 'pa11ycrawler']) +@cmdopts([BOKCHOY_DEFAULT_STORE, BOKCHOY_DEFAULT_STORE_DEPR]) @timed def load_bok_choy_data(options): """ @@ -48,7 +56,10 @@ def load_bok_choy_data(options): @task -@cmdopts(BOKCHOY_OPTS, share_with=['test_bokchoy', 'test_a11y', 'pa11ycrawler']) +@cmdopts([ + BOKCHOY_IMPORTS_DIR, BOKCHOY_IMPORTS_DIR_DEPR, BOKCHOY_DEFAULT_STORE, + BOKCHOY_DEFAULT_STORE_DEPR +]) @timed def load_courses(options): """ @@ -70,6 +81,51 @@ def load_courses(options): import_dir=options.imports_dir ) ) + else: + print colorize('blue', "--imports-dir not set, skipping import") + + +@task +@cmdopts([BOKCHOY_IMPORTS_DIR, BOKCHOY_IMPORTS_DIR_DEPR, PA11Y_FETCH_COURSE]) +@timed +def get_test_course(options): + """ + Fetches the test course. + """ + + if options.get('imports_dir'): + print colorize("green", "--imports-dir specified, skipping fetch of test course") + return + + if not options.get('should_fetch_course', False): + print colorize("green", "--skip-fetch specified, skipping fetch of test course") + return + + # Set the imports_dir for use by other tasks + options.imports_dir = DEMO_COURSE_IMPORT_DIR + + options.imports_dir.makedirs_p() + zipped_course = options.imports_dir + 'demo_course.tar.gz' + + msg = colorize('green', "Fetching the test course from github...") + print msg + + sh( + 'wget {tar_gz_file} -O {zipped_course}'.format( + tar_gz_file=DEMO_COURSE_TAR_GZ, + zipped_course=zipped_course, + ) + ) + + msg = colorize('green', "Uncompressing the test course...") + print msg + + sh( + 'tar zxf {zipped_course} -C {courses_dir}'.format( + zipped_course=zipped_course, + courses_dir=options.imports_dir, + ) + ) @task @@ -83,7 +139,8 @@ def reset_test_database(): @task @needs(['reset_test_database', 'clear_mongo', 'load_bok_choy_data', 'load_courses']) -@cmdopts(BOKCHOY_OPTS, share_with=['test_bokchoy', 'test_a11y', 'pa11ycrawler']) +@might_call('start_servers') +@cmdopts([BOKCHOY_FASTTEST], share_with=['start_servers']) @timed def prepare_bokchoy_run(options): """ @@ -289,21 +346,7 @@ class Pa11yCrawler(BokChoyTestSuite): def __init__(self, *args, **kwargs): super(Pa11yCrawler, self).__init__(*args, **kwargs) self.course_key = kwargs.get('course_key') - if self.imports_dir: - # If imports_dir has been specified, assume the files are - # already there -- no need to fetch them from github. This - # allows someome to crawl a different course. They are responsible - # for putting it, un-archived, in the directory. - self.should_fetch_course = False - else: - # Otherwise, obey `--skip-fetch` command and use the default - # test course. Note that the fetch will also be skipped when - # using `--fast`. - self.should_fetch_course = kwargs.get('should_fetch_course') - self.imports_dir = path('test_root/courses/') - self.pa11y_report_dir = os.path.join(self.report_dir, 'pa11ycrawler_reports') - self.tar_gz_file = "https://github.com/edx/demo-test-course/archive/master.tar.gz" self.start_urls = [] auto_auth_params = { @@ -325,38 +368,6 @@ class Pa11yCrawler(BokChoyTestSuite): lms_params = urlencode(auto_auth_params) self.start_urls.append("\"http://localhost:8003/auto_auth?{}\"".format(lms_params)) - def __enter__(self): - if self.should_fetch_course: - self.get_test_course() - super(Pa11yCrawler, self).__enter__() - - def get_test_course(self): - """ - Fetches the test course. - """ - self.imports_dir.makedirs_p() - zipped_course = self.imports_dir + 'demo_course.tar.gz' - - msg = colorize('green', "Fetching the test course from github...") - print msg - - sh( - 'wget {tar_gz_file} -O {zipped_course}'.format( - tar_gz_file=self.tar_gz_file, - zipped_course=zipped_course, - ) - ) - - msg = colorize('green', "Uncompressing the test course...") - print msg - - sh( - 'tar zxf {zipped_course} -C {courses_dir}'.format( - zipped_course=zipped_course, - courses_dir=self.imports_dir, - ) - ) - def generate_html_reports(self): """ Runs pa11ycrawler json-to-html