From 6e93cee2dad7deef01ae8d99ed51b77dfc2ccd2d Mon Sep 17 00:00:00 2001 From: Christine Lytwynec Date: Wed, 20 Apr 2016 16:46:24 -0400 Subject: [PATCH] add --skip-fetch flag, update start_urls, update auto_auth to include redirect_to param --- .../student/tests/test_auto_auth.py | 11 ++++ common/djangoapps/student/views.py | 13 +++-- pavelib/bok_choy.py | 20 +++++-- .../paver_tests/test_paver_bok_choy_cmds.py | 33 ++++------- pavelib/utils/test/suites/__init__.py | 2 +- pavelib/utils/test/suites/bokchoy_suite.py | 55 ++++++++++++++----- 6 files changed, 87 insertions(+), 47 deletions(-) diff --git a/common/djangoapps/student/tests/test_auto_auth.py b/common/djangoapps/student/tests/test_auto_auth.py index 6b9c942f7d..acc17f19cb 100644 --- a/common/djangoapps/student/tests/test_auto_auth.py +++ b/common/djangoapps/student/tests/test_auto_auth.py @@ -219,6 +219,17 @@ class AutoAuthEnabledTestCase(UrlResetMixin, TestCase): self.assertTrue(response.url.endswith(url_pattern)) # pylint: disable=no-member + def test_redirect_to_specified(self): + # Create user and redirect to specified url + url_pattern = '/u/test#about_me' + response = self._auto_auth({ + 'username': 'test', + 'redirect_to': url_pattern, + 'staff': 'true', + }, status_code=302) + + self.assertTrue(response.url.endswith(url_pattern)) # pylint: disable=no-member + def _auto_auth(self, params=None, status_code=None, **kwargs): """ Make a request to the auto-auth end-point and check diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 74067e0dc4..983e9284f5 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1906,8 +1906,9 @@ def auto_auth(request): * `course_id`: Enroll the student in the course with `course_id` * `roles`: Comma-separated list of roles to grant the student in the course with `course_id` * `no_login`: Define this to create the user but not login - * `redirect`: Set to "true" will redirect to course if course_id is defined, otherwise it will redirect to dashboard - + * `redirect`: Set to "true" will redirect to the `redirect_to` value if set, or + course home page if course_id is defined, otherwise it will redirect to dashboard + * `redirect_to`: will redirect to to this url If username, email, or password are not provided, use randomly generated credentials. """ @@ -1923,6 +1924,7 @@ def auto_auth(request): is_staff = request.GET.get('staff', None) is_superuser = request.GET.get('superuser', None) course_id = request.GET.get('course_id', None) + redirect_to = request.GET.get('redirect_to', None) # mode has to be one of 'honor'/'professional'/'verified'/'audit'/'no-id-professional'/'credit' enrollment_mode = request.GET.get('enrollment_mode', 'honor') @@ -1931,7 +1933,7 @@ def auto_auth(request): if course_id: course_key = CourseLocator.from_string(course_id) role_names = [v.strip() for v in request.GET.get('roles', '').split(',') if v.strip()] - redirect_when_done = request.GET.get('redirect', '').lower() == 'true' + redirect_when_done = request.GET.get('redirect', '').lower() == 'true' or redirect_to login_when_done = 'no_login' not in request.GET form = AccountCreationForm( @@ -1996,8 +1998,11 @@ def auto_auth(request): # Provide the user with a valid CSRF token # then return a 200 response unless redirect is true if redirect_when_done: + # Redirect to specific page if specified + if redirect_to: + redirect_url = redirect_to # Redirect to course info page if course_id is known - if course_id: + elif course_id: try: # redirect to course info page in LMS redirect_url = reverse( diff --git a/pavelib/bok_choy.py b/pavelib/bok_choy.py index 823882a9b4..1a206c1aa5 100644 --- a/pavelib/bok_choy.py +++ b/pavelib/bok_choy.py @@ -3,7 +3,7 @@ Run acceptance tests that use the bok-choy framework http://bok-choy.readthedocs.org/en/latest/ """ from paver.easy import task, needs, cmdopts, sh -from pavelib.utils.test.suites.bokchoy_suite import BokChoyTestSuite, A11yCrawler +from pavelib.utils.test.suites.bokchoy_suite import BokChoyTestSuite, Pa11yCrawler from pavelib.utils.envs import Env from pavelib.utils.test.utils import check_firefox_version from optparse import make_option @@ -24,6 +24,7 @@ BOKCHOY_OPTS = [ ('extra_args=', 'e', 'adds as extra args to the test command'), ('default_store=', 's', 'Default modulestore'), ('test_dir=', 'd', 'Directory for finding tests (relative to common/test/acceptance)'), + ('imports_dir=', 'i', 'Directory containing (un-archived) courses to be imported'), ('num_processes=', 'n', 'Number of test threads (for multiprocessing)'), ('verify_xss', 'x', 'Run XSS vulnerability tests'), make_option("--verbose", action="store_const", const=2, dest="verbosity"), @@ -53,6 +54,7 @@ def parse_bokchoy_opts(options): 'extra_args': getattr(options, 'extra_args', ''), 'pdb': getattr(options, 'pdb', False), 'test_dir': getattr(options, 'test_dir', 'tests'), + 'imports_dir': getattr(options, 'imports_dir', None), 'save_screenshots': getattr(options, 'save_screenshots', False), } @@ -115,15 +117,12 @@ def test_a11y(options): @task @needs('pavelib.prereqs.install_prereqs') -@cmdopts(BOKCHOY_OPTS + [ - ('imports_dir=', 'd', 'Directory containing (un-archived) courses to be imported'), -]) +@cmdopts(BOKCHOY_OPTS) def perf_report_bokchoy(options): """ Generates a har file for with page performance info. """ opts = parse_bokchoy_opts(options) - opts['imports_dir'] = getattr(options, 'imports_dir', None) opts['test_dir'] = 'performance' run_bokchoy(**opts) @@ -133,6 +132,13 @@ def perf_report_bokchoy(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( + "--skip-fetch", + action="store_false", + dest="should_fetch_course", + help='Course key for test course', + ), ]) def pa11ycrawler(options): """ @@ -146,7 +152,9 @@ def pa11ycrawler(options): opts = parse_bokchoy_opts(options) opts['report_dir'] = Env.PA11YCRAWLER_REPORT_DIR opts['coveragerc'] = Env.PA11YCRAWLER_COVERAGERC - test_suite = A11yCrawler('a11y_crawler', **opts) + opts['should_fetch_course'] = getattr(options, 'should_fetch_course', None) + opts['course_key'] = getattr(options, 'course-key', None) + test_suite = Pa11yCrawler('a11y_crawler', **opts) test_suite.run() if getattr(options, 'with_html', False): diff --git a/pavelib/paver_tests/test_paver_bok_choy_cmds.py b/pavelib/paver_tests/test_paver_bok_choy_cmds.py index 88fd4d2040..d9290c017f 100644 --- a/pavelib/paver_tests/test_paver_bok_choy_cmds.py +++ b/pavelib/paver_tests/test_paver_bok_choy_cmds.py @@ -8,7 +8,7 @@ import unittest from mock import patch, call from test.test_support import EnvironmentVarGuard from paver.easy import BuildFailure -from pavelib.utils.test.suites import BokChoyTestSuite, A11yCrawler +from pavelib.utils.test.suites import BokChoyTestSuite, Pa11yCrawler REPO_DIR = os.getcwd() @@ -171,7 +171,7 @@ class TestPaverBokChoyCmd(unittest.TestCase): BokChoyTestSuite.verbosity_processes_string(suite) -class TestPaverPa11ycrawlerCmd(unittest.TestCase): +class TestPaverPa11yCrawlerCmd(unittest.TestCase): """ Paver pa11ycrawler command test cases. Most of the functionality is @@ -179,7 +179,7 @@ class TestPaverPa11ycrawlerCmd(unittest.TestCase): """ def setUp(self): - super(TestPaverPa11ycrawlerCmd, self).setUp() + super(TestPaverPa11yCrawlerCmd, self).setUp() # Mock shell commands mock_sh = patch('pavelib.utils.test.suites.bokchoy_suite.sh') @@ -188,41 +188,32 @@ class TestPaverPa11ycrawlerCmd(unittest.TestCase): # Cleanup mocks self.addCleanup(mock_sh.stop) - def _expected_command(self, report_dir): + def _expected_command(self, report_dir, start_urls): """ Returns the expected command to run pa11ycrawler. """ - cms_start_url = ( - "http://localhost:8031/auto_auth?redirect=true&course_id=course-v1" - "%3AedX%2BTest101%2Bcourse&staff=true" - ) - - lms_start_url = ( - "http://localhost:8003/auto_auth?redirect=true&course_id=course-v1" - "%3AedX%2BTest101%2Bcourse&staff=true" - ) - expected_statement = ( - 'pa11ycrawler run "{cms_start_url}" "{lms_start_url}" ' + 'pa11ycrawler run {start_urls} ' '--pa11ycrawler-allowed-domains=localhost ' '--pa11ycrawler-reports-dir={report_dir} ' '--pa11ycrawler-deny-url-matcher=logout ' '--pa11y-reporter="1.0-json" ' '--depth-limit=6 ' ).format( - cms_start_url=cms_start_url, - lms_start_url=lms_start_url, + start_urls=start_urls, report_dir=report_dir, ) return expected_statement def test_default(self): - suite = A11yCrawler('') + suite = Pa11yCrawler('') self.assertEqual( - suite.cmd, self._expected_command(suite.pa11y_report_dir)) + suite.cmd, + self._expected_command(suite.pa11y_report_dir, suite.start_urls) + ) def test_get_test_course(self): - suite = A11yCrawler('') + suite = Pa11yCrawler('') suite.get_test_course() self._mock_sh.assert_has_calls([ call( @@ -232,7 +223,7 @@ class TestPaverPa11ycrawlerCmd(unittest.TestCase): ]) def test_generate_html_reports(self): - suite = A11yCrawler('') + suite = Pa11yCrawler('') suite.generate_html_reports() self._mock_sh.assert_has_calls([ call( diff --git a/pavelib/utils/test/suites/__init__.py b/pavelib/utils/test/suites/__init__.py index 0900f994fd..5617a12900 100644 --- a/pavelib/utils/test/suites/__init__.py +++ b/pavelib/utils/test/suites/__init__.py @@ -6,4 +6,4 @@ from .nose_suite import NoseTestSuite, SystemTestSuite, LibTestSuite from .python_suite import PythonTestSuite from .js_suite import JsTestSuite from .acceptance_suite import AcceptanceTestSuite -from .bokchoy_suite import BokChoyTestSuite, A11yCrawler +from .bokchoy_suite import BokChoyTestSuite, Pa11yCrawler diff --git a/pavelib/utils/test/suites/bokchoy_suite.py b/pavelib/utils/test/suites/bokchoy_suite.py index 4ec353626e..fc6a66a407 100644 --- a/pavelib/utils/test/suites/bokchoy_suite.py +++ b/pavelib/utils/test/suites/bokchoy_suite.py @@ -260,22 +260,55 @@ class BokChoyTestSuite(TestSuite): return cmd -class A11yCrawler(BokChoyTestSuite): +class Pa11yCrawler(BokChoyTestSuite): """ Sets up test environment with mega-course loaded, and runs pa11ycralwer against it. """ def __init__(self, *args, **kwargs): - super(A11yCrawler, self).__init__(*args, **kwargs) + super(Pa11yCrawler, self).__init__(*args, **kwargs) + self.course_key = kwargs.get('course_key', "course-v1:edX+Test101+course") + 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', not self.fasttest) + self.imports_dir = path('test_root/courses/') self.pa11y_report_dir = os.path.join(self.report_dir, 'pa11ycrawler_reports') - self.imports_dir = path('test_root/courses/') self.tar_gz_file = "https://github.com/edx/demo-test-course/archive/master.tar.gz" + self.start_urls = [] + auto_auth_params = { + "redirect": 'true', + "staff": 'true', + "course_id": self.course_key, + } + cms_params = urlencode(auto_auth_params) + self.start_urls.append("\"http://localhost:8031/auto_auth?{}\"".format(cms_params)) + + sequence_url = "/api/courses/v1/blocks/?{}".format( + urlencode({ + "course_id": self.course_key, + "depth": "all", + "all_blocks": "true", + }) + ) + auto_auth_params.update({'redirect_to': sequence_url}) + lms_params = urlencode(auto_auth_params) + self.start_urls.append("\"http://localhost:8003/auto_auth?{}\"".format(lms_params)) + def __enter__(self): - self.get_test_course() - super(A11yCrawler, self).__enter__() + if self.should_fetch_course: + self.get_test_course() + super(Pa11yCrawler, self).__enter__() def get_test_course(self): """ @@ -319,23 +352,15 @@ class A11yCrawler(BokChoyTestSuite): """ Runs pa11ycrawler as staff user against the test course. """ - params = urlencode({ - "redirect": 'true', - "staff": 'true', - "course_id": "course-v1:edX+Test101+course", - }) - cms_start_url = 'http://localhost:8031/auto_auth?{}'.format(params) - lms_start_url = 'http://localhost:8003/auto_auth?{}'.format(params) cmd_str = ( - 'pa11ycrawler run "{cms_start_url}" "{lms_start_url}" ' + 'pa11ycrawler run {start_urls} ' '--pa11ycrawler-allowed-domains={allowed_domains} ' '--pa11ycrawler-reports-dir={report_dir} ' '--pa11ycrawler-deny-url-matcher={dont_go_here} ' '--pa11y-reporter="{reporter}" ' '--depth-limit={depth} ' ).format( - cms_start_url=cms_start_url, - lms_start_url=lms_start_url, + start_urls=self.start_urls, allowed_domains='localhost', report_dir=self.pa11y_report_dir, reporter="1.0-json",