Merge pull request #1 from edx/clintonb/updates

Multiple infrastructure updates
openedx · Jan 27, 2016 · 45a4413 · 45a4413
2 parents e6b0370 + bc084d6
commit 45a4413
Show file tree

Hide file tree

Showing 15 changed files with 223 additions and 388 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,12 +1,11 @@
 language: python
 python:
   - "2.7"
-before_install:
-  - git fetch origin master:refs/remotes/origin/master
 install:
-  - pip install -r test-requirements.txt
+  - make requirements
   - pip install coveralls
 script:
-  - scripts/coverage
+  - make test
+  - make quality
 after_success:
   - coveralls
diff --git a/Makefile b/Makefile
@@ -0,0 +1,16 @@
+.DEFAULT_GOAL := test
+
+.PHONY: html_coverage, quality, requirements
+
+html_coverage:
+	coverage html && open htmlcov/index.html
+
+quality:
+	pep8 --config=.pep8 ccx_keys
+	pylint --rcfile=pylintrc ccx_keys
+
+requirements:
+	pip install -r requirements.txt
+
+test:
+	coverage run -m nose
diff --git a/README.rst b/README.rst
@@ -5,15 +5,14 @@ Part of `edX code`_.
 ccx-keys |build-status| |coverage-status|
 =========================================
 
-Ccx-keys provides implementations of the Opaque Key concept specific to Custom
-Courses for EdX.  For more on opaque keys see `the opaque-keys package`_, 
-`its documentation`_, or the `edx-platform wiki documentation`_ regarding 
-opaque keys.
+ccx-keys provides implementations of the Opaque Key concept specific to Custom Courses for EdX.
+For more on opaque keys see `the opaque-keys package`_, `its documentation`_, or
+the `edx-platform wiki documentation`_ regarding opaque keys.
 
-.. |build-status| image:: https://travis-ci.org/jazkarta/ccx-keys.png  
-   :target: https://travis-ci.org/jazkarta/ccx-keys
-.. |coverage-status| image:: https://coveralls.io/repos/jazkarta/ccx-keys/badge.svg
-   :target: https://coveralls.io/r/jazkarta/ccx-keys
+.. |build-status| image:: https://travis-ci.org/edx/ccx-keys.svg?branch=master
+   :target: https://travis-ci.org/edx/ccx-keys
+.. |coverage-status| image:: https://coveralls.io/repos/edx/ccx-keys/badge.svg
+   :target: https://coveralls.io/r/edx/ccx-keys
 
 .. _`the opaque-keys package`: https://github.com/edx/opaque-keys
 .. _`its documentation`: http://opaque-keys.readthedocs.org/en/latest/

diff --git a/ccx_keys/key.py b/ccx_keys/key.py
@@ -1,9 +1,12 @@
-# -*- coding: utf-8 -*-
+""" Key module. """
+
 from abc import abstractproperty
+
 from opaque_keys.edx.keys import CourseKey
 
 
 class CCXKey(CourseKey):
+    """ Custom course key. """
 
     @abstractproperty
     def ccx(self):

diff --git a/ccx_keys/locator.py b/ccx_keys/locator.py
@@ -1,13 +1,14 @@
 # -*- coding: utf-8 -*-
+""" Locator module. """
 import re
 
 from opaque_keys import InvalidKeyError
+from opaque_keys.edx.keys import UsageKey
 from opaque_keys.edx.locator import (
     AssetLocator,
     BlockUsageLocator,
     CourseLocator,
 )
-from opaque_keys.edx.keys import UsageKey
 
 from ccx_keys.key import CCXKey
 
@@ -16,7 +17,7 @@ class CCXLocator(CourseLocator, CCXKey):
     """Concrete implementation of an Opaque Key for CCX courses"""
 
     CANONICAL_NAMESPACE = 'ccx-v1'
-    KEY_FIELDS = CourseLocator.KEY_FIELDS + ('ccx', )
+    KEY_FIELDS = CourseLocator.KEY_FIELDS + ('ccx',)
     __slots__ = KEY_FIELDS
     CHECKED_INIT = False
     CCX_PREFIX = 'ccx'
@@ -72,6 +73,10 @@ def from_course_locator(cls, course_locator, ccx):
         return new_obj
 
     def to_course_locator(self):
+        """
+        Returns a CourseLocator representing this location.
+        """
+        # pylint: disable=no-member
         return CourseLocator(
             org=self.org,
             course=self.course,
@@ -120,7 +125,7 @@ def make_asset_key(self, asset_type, path):
 class CCXBlockUsageLocator(BlockUsageLocator, UsageKey):
     """Concrete implementation of a usage key for blocks in CCXs"""
 
-    CANONICAL_NAMESPACE = 'ccx-block-v1'    
+    CANONICAL_NAMESPACE = 'ccx-block-v1'
 
     URL_RE = re.compile(
         '^' + CCXLocator.URL_RE_SOURCE + '$', re.IGNORECASE | re.VERBOSE | re.UNICODE
@@ -158,6 +163,9 @@ def ccx(self):
         return self.course_key.ccx
 
     def to_block_locator(self):
+        """
+        Returns a BlockUsageLocator for this location.
+        """
         return BlockUsageLocator(
             course_key=self.course_key.to_course_locator(),
             block_type=self.block_type,

diff --git a/ccx_keys/tests/test_ccx_keys.py b/ccx_keys/tests/test_ccx_keys.py
@@ -1,13 +1,13 @@
+""" Tests for the ccx_keys package. """
+
 import ddt
 from bson.objectid import ObjectId
-from itertools import product
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey, UsageKey
 from opaque_keys.edx.locator import CourseLocator
 from opaque_keys.edx.tests import LocatorBaseTest, TestDeprecated
 
-from ccx_keys.locator import CCXLocator
-from ccx_keys.locator import CCXBlockUsageLocator
+from ccx_keys.locator import CCXBlockUsageLocator, CCXLocator
 
 
 @ddt.ddt
@@ -137,27 +137,34 @@ def test_missing_ccx_id(self, fields):
         use_fields = dict(
             (k, v) for k, v in available_fields.items() if k in fields
         )
-        with self.assertRaises(InvalidKeyError) as cm:
+        with self.assertRaises(InvalidKeyError) as context_manager:
             CCXLocator(**use_fields)
 
-        self.assertTrue(str(CCXLocator) in str(cm.exception))
+        self.assertIn(str(CCXLocator), str(context_manager.exception))
 
+    # pylint: disable=line-too-long
     @ddt.unpack
     @ddt.data(
-        {'fields': ('version_guid',),
-         'url_template': '{CANONICAL_NAMESPACE}:{VERSION_PREFIX}@{version_guid}+{CCX_PREFIX}@{ccx}',
-         },
-        {'fields': ('org', 'course', 'run'),
-         'url_template': '{CANONICAL_NAMESPACE}:{org}+{course}+{run}+{CCX_PREFIX}@{ccx}',
-         },
-        {'fields': ('org', 'course', 'run', 'branch'),
-         'url_template': '{CANONICAL_NAMESPACE}:{org}+{course}+{run}+{BRANCH_PREFIX}@{branch}+{CCX_PREFIX}@{ccx}',
-         },
-        {'fields': ('org', 'course', 'run', 'version_guid'),
-         'url_template': '{CANONICAL_NAMESPACE}:{org}+{course}+{run}+{VERSION_PREFIX}@{version_guid}+{CCX_PREFIX}@{ccx}',
-         },
-        {'fields': ('org', 'course', 'run', 'branch', 'version_guid'),
-         'url_template': '{CANONICAL_NAMESPACE}:{org}+{course}+{run}+{BRANCH_PREFIX}@{branch}+{VERSION_PREFIX}@{version_guid}+{CCX_PREFIX}@{ccx}',},
+        {
+            'fields': ('version_guid',),
+            'url_template': '{CANONICAL_NAMESPACE}:{VERSION_PREFIX}@{version_guid}+{CCX_PREFIX}@{ccx}',
+        },
+        {
+            'fields': ('org', 'course', 'run'),
+            'url_template': '{CANONICAL_NAMESPACE}:{org}+{course}+{run}+{CCX_PREFIX}@{ccx}',
+        },
+        {
+            'fields': ('org', 'course', 'run', 'branch'),
+            'url_template': '{CANONICAL_NAMESPACE}:{org}+{course}+{run}+{BRANCH_PREFIX}@{branch}+{CCX_PREFIX}@{ccx}',
+        },
+        {
+            'fields': ('org', 'course', 'run', 'version_guid'),
+            'url_template': '{CANONICAL_NAMESPACE}:{org}+{course}+{run}+{VERSION_PREFIX}@{version_guid}+{CCX_PREFIX}@{ccx}',
+        },
+        {
+            'fields': ('org', 'course', 'run', 'branch', 'version_guid'),
+            'url_template': '{CANONICAL_NAMESPACE}:{org}+{course}+{run}+{BRANCH_PREFIX}@{branch}+{VERSION_PREFIX}@{version_guid}+{CCX_PREFIX}@{ccx}',
+        },
     )
     def test_locator_from_good_url(self, fields, url_template):
         available_fields = {
@@ -236,14 +243,20 @@ class TestCCXBlockUsageLocator(LocatorBaseTest):
     """
     Tests of :class:`.CCXBlockUsageLocator`
     """
+
     @ddt.data(
         # do we need or even want to support deprecated forms of urls?
-        "ccx-block-v1:org+course+run+ccx@1+{}@category+{}@name".format(CCXBlockUsageLocator.BLOCK_TYPE_PREFIX, CCXBlockUsageLocator.BLOCK_PREFIX),
-        "ccx-block-v1:org+course+run+{}@revision+ccx@1+{}@category+{}@name".format(CourseLocator.BRANCH_PREFIX, CCXBlockUsageLocator.BLOCK_TYPE_PREFIX, CCXBlockUsageLocator.BLOCK_PREFIX),
+        "ccx-block-v1:org+course+run+ccx@1+{}@category+{}@name".format(CCXBlockUsageLocator.BLOCK_TYPE_PREFIX,
+                                                                       CCXBlockUsageLocator.BLOCK_PREFIX),
+        "ccx-block-v1:org+course+run+{}@revision+ccx@1+{}@category+{}@name".format(
+            CourseLocator.BRANCH_PREFIX,
+            CCXBlockUsageLocator.BLOCK_TYPE_PREFIX,
+            CCXBlockUsageLocator.BLOCK_PREFIX),
         "i4x://org/course/category/name@revision",
         # now try the extended char sets - we expect that "%" should be OK in deprecated-style ids,
         # but should not be valid in new-style ids
-        "ccx-block-v1:org.dept.sub-prof+course.num.section-4+run.hour.min-99+ccx@1+{}@category+{}@name:12.33-44".format(CCXBlockUsageLocator.BLOCK_TYPE_PREFIX, CCXBlockUsageLocator.BLOCK_PREFIX),
+        "ccx-block-v1:org.dept.sub-prof+course.num.section-4+run.hour.min-99+ccx@1+{}@category+{}@name:12.33-44".format(
+            CCXBlockUsageLocator.BLOCK_TYPE_PREFIX, CCXBlockUsageLocator.BLOCK_PREFIX),
         "i4x://org.dept%sub-prof/course.num%section-4/category/name:12%33-44",
     )
     def test_string_roundtrip(self, url):
@@ -255,33 +268,19 @@ def test_string_roundtrip(self, url):
 
     @ddt.data(
         "ccx-block-v1:org+course+run+ccx@1+{}@category".format(CCXBlockUsageLocator.BLOCK_TYPE_PREFIX),
-        "ccx-block-v1:org+course+run+{}@revision+ccx@1+{}@category".format(CourseLocator.BRANCH_PREFIX, CCXBlockUsageLocator.BLOCK_TYPE_PREFIX),
+        "ccx-block-v1:org+course+run+{}@revision+ccx@1+{}@category".format(CourseLocator.BRANCH_PREFIX,
+                                                                           CCXBlockUsageLocator.BLOCK_TYPE_PREFIX),
     )
     def test_missing_block_id(self, url):
         with self.assertRaises(InvalidKeyError):
             UsageKey.from_string(url)
 
     @ddt.data(
-        ((), {
-            'org': 'org',
-            'course': 'course',
-            'run': 'run',
-            'ccx': '1',
-            'category': 'category',
-            'name': 'name',
-        }, 'org', 'course', 'run', '1', 'category', 'name', None),
-        ((), {
-            'org': 'org',
-            'course': 'course',
-            'run': 'run',
-            'ccx': '1',
-            'category': 'category',
-            'name': 'name:more_name',
-        }, 'org', 'course', 'run', '1', 'category', 'name:more_name', None),
-        ([], {}, 'org', 'course', 'run', '1', 'category', 'name', None),
+        ('org', 'course', 'run', '1', 'category', 'name', None),
+        ('org', 'course', 'run', '1', 'category', 'name:more_name', None),
     )
     @ddt.unpack
-    def test_valid_locations(self, args, kwargs, org, course, run, ccx, category, name, revision):  # pylint: disable=unused-argument
+    def test_valid_locations(self, org, course, run, ccx, category, name, revision):  # pylint: disable=unused-argument
         course_key = CCXLocator(org=org, course=course, run=run, branch=revision, ccx=ccx)
         locator = CCXBlockUsageLocator(course_key, block_type=category, block_id=name, )
         self.assertEquals(org, locator.org)
@@ -313,7 +312,7 @@ def test_valid_locations(self, args, kwargs, org, course, run, ccx, category, na
             'tag': 'tag',
             'course': 'course',
             'category': 'category',
-            'name': 'name ',   # extra space
+            'name': 'name ',  # extra space
             'org': 'org'
         }),
     )
@@ -322,8 +321,6 @@ def test_invalid_locations(self, *args, **kwargs):
         with self.assertRaises(TypeError):
             CCXBlockUsageLocator(*args, **kwargs)
 
-
-
     @ddt.data(
         ('course', 'newvalue'),
         ('org', 'newvalue'),