add more pearson tests, and update commands in response

common/djangoapps/student/management/commands/pearson_export_cdd.py

@@ -5,7 +5,7 @@ from datetime import datetime
 from optparse import make_option
 
 from django.conf import settings
-from django.core.management.base import BaseCommand
+from django.core.management.base import BaseCommand, CommandError
 
 from student.models import TestCenterUser
 
@@ -39,6 +39,9 @@ class Command(BaseCommand):
         ("LastUpdate", "user_updated_at"),  # in UTC, so same as what we store
     ])
 
+    # define defaults, even thought 'store_true' shouldn't need them.  
+    # (call_command will set None as default value for all options that don't have one,
+    # so one cannot rely on presence/absence of flags in that world.)
     option_list = BaseCommand.option_list + (
         make_option('--dest-from-settings',
                     action='store_true',
@@ -63,13 +66,13 @@ class Command(BaseCommand):
         # Name will use timestamp -- this is UTC, so it will look funny,
         # but it should at least be consistent with the other timestamps
         # used in the system.
-        if 'dest-from-settings' in options:
+        if 'dest-from-settings' in options and options['dest-from-settings']:
             if 'LOCAL_EXPORT' in settings.PEARSON:
                 dest = settings.PEARSON['LOCAL_EXPORT']
             else:
                 raise CommandError('--dest-from-settings was enabled but the'
                                    'PEARSON[LOCAL_EXPORT] setting was not set.')
-        elif 'destination' in options:
+        elif 'destination' in options and options['destination']:
             dest = options['destination']
         else:
             raise CommandError('--destination or --dest-from-settings must be used')

common/djangoapps/student/management/commands/pearson_export_ead.py

@@ -5,7 +5,7 @@ from datetime import datetime
 from optparse import make_option
 
 from django.conf import settings
-from django.core.management.base import BaseCommand
+from django.core.management.base import BaseCommand, CommandError
 
 from student.models import TestCenterRegistration
 
@@ -39,10 +39,12 @@ class Command(BaseCommand):
         make_option('--dump_all',
                     action='store_true',
                     dest='dump_all',
+                    default=False,
                     ),
         make_option('--force_add',
                     action='store_true',
                     dest='force_add',
+                    default=False,
                     ),
     )
 
@@ -57,13 +59,13 @@ class Command(BaseCommand):
         # Name will use timestamp -- this is UTC, so it will look funny,
         # but it should at least be consistent with the other timestamps
         # used in the system.
-        if 'dest-from-settings' in options:
+        if 'dest-from-settings' in options and options['dest-from-settings']:
             if 'LOCAL_EXPORT' in settings.PEARSON:
                 dest = settings.PEARSON['LOCAL_EXPORT']
             else:
                 raise CommandError('--dest-from-settings was enabled but the'
                                    'PEARSON[LOCAL_EXPORT] setting was not set.')
-        elif 'destinations' in options:
+        elif 'destination' in options and options['destination']:
             dest = options['destination']
         else:
             raise CommandError('--destination or --dest-from-settings must be used')

common/djangoapps/student/management/commands/pearson_transfer.py

+import os
 from optparse import make_option
 
+from django.conf import settings
 from django.core.management.base import BaseCommand, CommandError
+from django.core.management import call_command
 from dogapi import dog_http_api, dog_stats_api
 import paramiko
 import boto
-import os
 
 dog_http_api.api_key = settings.DATADOG_API
 
@@ -13,7 +15,7 @@ class Command(BaseCommand):
     help = """
     This command handles the importing and exporting of student records for
     Pearson.  It uses some other Django commands to export and import the
-    files and then uploads over SFTP to pearson and stuffs the entry in an
+    files and then uploads over SFTP to Pearson and stuffs the entry in an
     S3 bucket for archive purposes.
 
     Usage: django-admin.py pearson-transfer --mode [import|export|both]
@@ -29,11 +31,12 @@ class Command(BaseCommand):
 
     def handle(self, **options):
 
+        # TODO: this doesn't work. Need to check if it's a property.
         if not settings.PEARSON:
             raise CommandError('No PEARSON entries in auth/env.json.')
 
-        for value in ['LOCAL_IMPORT', 'SFTP_IMPORT', 'BUCKET', 'LOCAL_EXPORT',
-                      'SFTP_EXPORT']:
+        for value in ['LOCAL_IMPORT', 'SFTP_IMPORT', 'LOCAL_EXPORT',
+                      'SFTP_EXPORT', 'SFTP_HOSTNAME', 'SFTP_USERNAME', 'SFTP_PASSWORD']:
             if value not in settings.PEARSON:
                 raise CommandError('No entry in the PEARSON settings'
                                    '(env/auth.json) for {0}'.format(value))

common/djangoapps/student/management/commands/tests/test_pearson.py

@@ -10,7 +10,7 @@ import cStringIO
 import sys
 
 from django.test import TestCase
-from django.core import management
+from django.core.management import call_command
 
 from student.models import User, TestCenterRegistration, TestCenterUser, get_testcenter_registration
 
@@ -30,11 +30,11 @@ def create_tc_user(username):
                    'phone' : '252-1866',
                    'phone_country_code' : '1',
                     }
-    management.call_command('pearson_make_tc_user', username, **options)
+    call_command('pearson_make_tc_user', username, **options)
     return TestCenterUser.objects.get(user=user)
     
     
-def create_tc_registration(username, course_id, exam_code, accommodation_code):
+def create_tc_registration(username, course_id = 'org1/course1/term1', exam_code = 'exam1', accommodation_code = None):
     
     options = { 'exam_series_code' : exam_code,
                'eligibility_appointment_date_first' : '2013-01-01T00:00',
@@ -42,11 +42,55 @@ def create_tc_registration(username, course_id, exam_code, accommodation_code):
                'accommodation_code' : accommodation_code, 
                }
 
-    management.call_command('pearson_make_tc_registration', username, course_id, **options)
+    call_command('pearson_make_tc_registration', username, course_id, **options)
     user = User.objects.get(username=username)
     registrations = get_testcenter_registration(user, course_id, exam_code)
     return registrations[0]
+
+def get_error_string_for_management_call(*args, **options):
+    stdout_string = None
+    old_stdout = sys.stdout
+    old_stderr = sys.stderr
+    sys.stdout = cStringIO.StringIO()
+    sys.stderr = cStringIO.StringIO()
+    try:
+        call_command(*args, **options)
+    except BaseException, why1:
+        # The goal here is to catch CommandError calls.
+        # But these are actually translated into nice messages,
+        # and sys.exit(1) is then called.  For testing, we
+        # want to catch what sys.exit throws, and get the
+        # relevant text either from stdout or stderr. 
+        # TODO: this should really check to see that we
+        # arrived here because of a sys.exit(1).  Otherwise
+        # we should just raise the exception.
+        stdout_string = sys.stdout.getvalue()
+        stderr_string = sys.stderr.getvalue()
+    except Exception, why:
+        raise why
     
+    finally:
+        sys.stdout = old_stdout
+        sys.stderr = old_stderr
+            
+    if stdout_string is None:
+        raise Exception("Expected call to {} to fail, but it succeeded!".format(args[0]))            
+    return stdout_string, stderr_string
+        
+
+def get_file_info(dirpath):
+    filelist = os.listdir(dirpath)
+    print 'Files found: {}'.format(filelist)
+    numfiles = len(filelist)
+    if numfiles == 1:
+        filepath = os.path.join(dirpath, filelist[0])
+        with open(filepath, 'r') as cddfile:
+            filecontents = cddfile.readlines()
+            numlines = len(filecontents)
+            return filepath, numlines
+    else:
+        raise Exception("Expected to find a single file in {}, but found {}".format(dirpath,filelist))
+            
 class PearsonTestCase(TestCase):
     '''
     Base class for tests running Pearson-related commands
@@ -54,7 +98,9 @@ class PearsonTestCase(TestCase):
     import_dir = mkdtemp(prefix="import")
     export_dir = mkdtemp(prefix="export")
     
-    
+    def assertErrorContains(self, error_message, expected):
+        self.assertTrue(error_message.find(expected) >= 0, 'error message "{}" did not contain "{}"'.format(error_message, expected))
+        
     def tearDown(self):
         def delete_temp_dir(dirname):
             if os.path.exists(dirname):
@@ -67,14 +113,13 @@ class PearsonTestCase(TestCase):
         delete_temp_dir(self.export_dir)
 
     def test_missing_demographic_fields(self):
-        old_stdout = sys.stdout
-        sys.stdout = cStringIO.StringIO()
+        # We won't bother to test all details of form validation here.  
+        # It is enough to show that it works here, but deal with test cases for the form
+        # validation in the student tests, not these management tests.
         username = 'baduser'
         User.objects.create_user(username, '{}@edx.org'.format(username), 'fakepass')
         options = {}
-               
-        self.assertRaises(BaseException, management.call_command, 'pearson_make_tc_user', username, **options)
-        output_string = sys.stdout.getvalue()
+        output_string, _ = get_error_string_for_management_call('pearson_make_tc_user', username, **options) 
         self.assertTrue(output_string.find('Field Form errors encountered:') >= 0)
         self.assertTrue(output_string.find('Field Form Error:  city') >= 0)
         self.assertTrue(output_string.find('Field Form Error:  first_name') >= 0)
@@ -83,35 +128,114 @@ class PearsonTestCase(TestCase):
         self.assertTrue(output_string.find('Field Form Error:  phone_country_code') >= 0)
         self.assertTrue(output_string.find('Field Form Error:  phone') >= 0)
         self.assertTrue(output_string.find('Field Form Error:  address_1') >= 0)
-        sys.stdout = old_stdout
+        self.assertErrorContains(output_string, 'Field Form Error:  address_1')
         
     def test_create_good_testcenter_user(self):
         testcenter_user = create_tc_user("test1")
+        self.assertIsNotNone(testcenter_user)
         
     def test_create_good_testcenter_registration(self):
         username = 'test1'
-        course_id = 'org1/course1/term1'
-        exam_code = 'exam1'
-        accommodation_code = 'NONE'
-        testcenter_user = create_tc_user(username)
-        registration = create_tc_registration(username, course_id, exam_code, accommodation_code)
-        
-    def test_export(self):
-        username = 'test1'
-        course_id = 'org1/course1/term1'
-        exam_code = 'exam1'
-        accommodation_code = 'NONE'
-        testcenter_user = create_tc_user(username)
-        registration = create_tc_registration(username, course_id, exam_code, accommodation_code)
-        #options = { 'destination' : self.export_dir }
-        options = { '--dest-from-settings' : None }
+        create_tc_user(username)
+        registration = create_tc_registration(username)
+        self.assertIsNotNone(registration)
+
+    def test_cdd_missing_option(self):
+        _, error_string = get_error_string_for_management_call('pearson_export_cdd', **{})
+        self.assertErrorContains(error_string, 'Error: --destination or --dest-from-settings must be used')
+   
+    def test_ead_missing_option(self):
+        _, error_string = get_error_string_for_management_call('pearson_export_ead', **{})
+        self.assertErrorContains(error_string, 'Error: --destination or --dest-from-settings must be used')
+
+    def test_export_single_cdd(self):
+        # before we generate any tc_users, we expect there to be nothing to output:
+        options = { 'dest-from-settings' : True }
         with self.settings(PEARSON={ 'LOCAL_EXPORT' : self.export_dir }):
-            management.call_command('pearson_export_cdd', **options)
-            print 'Files found: {}'.format(os.listdir(self.export_dir))
-            self.assertEquals(len(os.listdir(self.export_dir)), 1, "Expect cdd file to be created")
-            management.call_command('pearson_export_ead', **options)
-            print 'Files found: {}'.format(os.listdir(self.export_dir))
-            self.assertEquals(len(os.listdir(self.export_dir)), 2, "Expect ead file to also be created")
-
-        # TODO: check that files were output....
+            call_command('pearson_export_cdd', **options)
+            (filepath, numlines) = get_file_info(self.export_dir)
+            self.assertEquals(numlines, 1, "Expect cdd file to have no non-header lines")
+            os.remove(filepath)
+
+            # generating a tc_user should result in a line in the output    
+            username = 'test_single_cdd'
+            create_tc_user(username)
+            call_command('pearson_export_cdd', **options)
+            (filepath, numlines) = get_file_info(self.export_dir)
+            self.assertEquals(numlines, 2, "Expect cdd file to have one non-header line")
+            os.remove(filepath)
+
+            # output after registration should not have any entries again.
+            call_command('pearson_export_cdd', **options)
+            (filepath, numlines) = get_file_info(self.export_dir)
+            self.assertEquals(numlines, 1, "Expect cdd file to have no non-header lines")
+            os.remove(filepath)
+
+            # if we modify the record, then it should be output again:
+            user_options = { 'first_name' : 'NewTestFirst', }
+            call_command('pearson_make_tc_user', username, **user_options)
+            call_command('pearson_export_cdd', **options)
+            (filepath, numlines) = get_file_info(self.export_dir)
+            self.assertEquals(numlines, 2, "Expect cdd file to have one non-header line")
+            os.remove(filepath)
+            
+    def test_export_single_ead(self):
+        # before we generate any registrations, we expect there to be nothing to output:
+        options = { 'dest-from-settings' : True }
+        with self.settings(PEARSON={ 'LOCAL_EXPORT' : self.export_dir }):
+            call_command('pearson_export_ead', **options)
+            (filepath, numlines) = get_file_info(self.export_dir)
+            self.assertEquals(numlines, 1, "Expect ead file to have no non-header lines")
+            os.remove(filepath)
+
+            # generating a registration should result in a line in the output    
+            username = 'test_single_ead'
+            create_tc_user(username)
+            create_tc_registration(username)
+            call_command('pearson_export_ead', **options)
+            (filepath, numlines) = get_file_info(self.export_dir)
+            self.assertEquals(numlines, 2, "Expect ead file to have one non-header line")
+            os.remove(filepath)
+
+            # output after registration should not have any entries again.
+            call_command('pearson_export_ead', **options)
+            (filepath, numlines) = get_file_info(self.export_dir)
+            self.assertEquals(numlines, 1, "Expect ead file to have no non-header lines")
+            os.remove(filepath)
         
+            # if we modify the record, then it should be output again:
+            create_tc_registration(username, accommodation_code='EQPMNT')
+            call_command('pearson_export_ead', **options)
+            (filepath, numlines) = get_file_info(self.export_dir)
+            self.assertEquals(numlines, 2, "Expect ead file to have one non-header line")
+            os.remove(filepath)
+
+    def test_export_multiple(self):
+        username1 = 'test_multiple1'
+        create_tc_user(username1)
+        create_tc_registration(username1)
+        create_tc_registration(username1, course_id = 'org1/course2/term1')
+        create_tc_registration(username1, exam_code = 'exam2')
+        username2 = 'test_multiple2'
+        create_tc_user(username2)
+        create_tc_registration(username2)
+        username3 = 'test_multiple3'
+        create_tc_user(username3)
+        create_tc_registration(username3, course_id = 'org1/course2/term1')
+        username4 = 'test_multiple4'
+        create_tc_user(username4)
+        create_tc_registration(username4, exam_code = 'exam2')
+
+        with self.settings(PEARSON={ 'LOCAL_EXPORT' : self.export_dir }):
+            options = { 'dest-from-settings' : True }
+            call_command('pearson_export_cdd', **options)
+            (filepath, numlines) = get_file_info(self.export_dir)
+            self.assertEquals(numlines, 5, "Expect cdd file to have four non-header lines: total was {}".format(numlines))
+            os.remove(filepath)
+
+            call_command('pearson_export_ead', **options)
+            (filepath, numlines) = get_file_info(self.export_dir)
+            self.assertEquals(numlines, 7, "Expect ead file to have six non-header lines: total was {}".format(numlines))
+            os.remove(filepath)
+
+