fix up tests

app/main/views/sms.py

@@ -44,7 +44,6 @@ message_templates = [
 @login_required
 def sendsms(service_id):
     form = CsvUploadForm()
-
     if form.validate_on_submit():
         try:
             csv_file = form.file.data
@@ -98,7 +97,7 @@ def checksms(service_id):
             # TODO when job is created record filename in job itself
             # so downstream pages can get the original filename that way
             session[upload_id] = filename
-            return redirect(url_for('.showjob', service_id=service_id, job_id=upload_id))
+            return redirect(url_for('main.showjob', service_id=service_id, job_id=upload_id))
         except:
             flash('There as a problem saving the file')
             return redirect(url_for('.checksms', recipients=filename))
@@ -118,10 +117,11 @@ def _format_filename(filename):
 
 
 def _build_upload_result(csv_file):
-    pattern = re.compile(r'^\+44\s?\d{4}\s?\d{6}$')
+    try:
-    with open(csv_file) as f:
+        file = open(csv_file, 'r')
+        pattern = re.compile(r'^\+44\s?\d{4}\s?\d{6}$')
         reader = csv.DictReader(
-            f,
+            file.read().splitlines(),
             lineterminator='\n',
             quoting=csv.QUOTE_NONE)
         valid, rejects = [], []
@@ -130,4 +130,6 @@ def _build_upload_result(csv_file):
                 valid.append(row)
             else:
                 rejects.append({"line_number": i+2, "phone": row['phone']})
-    return {"valid": valid, "rejects": rejects}
+        return {"valid": valid, "rejects": rejects}
+    finally:
+        file.close()

tests/app/main/views/test_sms.py

@@ -1,16 +1,12 @@
 from io import BytesIO
+from unittest import mock
+from unittest.mock import mock_open
+
 from tests.app.main import create_test_user
 
 
-def teardown_function(function):
+def test_upload_empty_csvfile_returns_to_upload_page(notifications_admin, notifications_admin_db, notify_db_session, mocker):
-    import glob
+    _setup_mocker_for_empty_file(mocker)
-    import os
-    files = glob.glob("/tmp/*.csv")
-    for f in files:
-        os.remove(f)
-
-
-def test_upload_empty_csvfile_returns_to_upload_page(notifications_admin, notifications_admin_db, notify_db_session):
     with notifications_admin.test_request_context():
         with notifications_admin.test_client() as client:
             user = create_test_user('active')
@@ -25,19 +21,26 @@ def test_upload_empty_csvfile_returns_to_upload_page(notifications_admin, notifi
 
 
 def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors(
-        notifications_admin, notifications_admin_db, notify_db_session):
+        notifications_admin, notifications_admin_db, notify_db_session,
+        mocker):
+
+    contents = 'phone\n+44 123\n+44 456'
+    file_data = (BytesIO(contents.encode('utf-8')), 'invalid.csv')
+    m_open = mock_open(read_data=contents)
+    _setup_mocker_for_nonemtpy_file(mocker)
 
     with notifications_admin.test_request_context():
         with notifications_admin.test_client() as client:
             user = create_test_user('active')
             client.login(user)
-
+            upload_data = {'file': file_data}
-            file_contents = 'phone\n+44 123\n+44 456'.encode('utf-8')
+            with mock.patch('app.main.views.sms.open', m_open):
-            upload_data = {'file': (BytesIO(file_contents), 'invalid.csv')}
+                response = client.post('/services/123/sms/send',
-            response = client.post('/services/123/sms/send', data=upload_data,
+                                       data=upload_data,
-                                   follow_redirects=True)
+                                       follow_redirects=True)
         assert response.status_code == 200
         content = response.get_data(as_text=True)
+
         assert 'There was a problem with some of the numbers' in content
         assert 'The following numbers are invalid' in content
         assert '+44 123' in content
@@ -46,17 +49,24 @@ def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors(
 
 
 def test_upload_csvfile_with_valid_phone_shows_first3_and_last3_numbers(
-        notifications_admin, notifications_admin_db, notify_db_session):
+        notifications_admin, notifications_admin_db, notify_db_session,
+        mocker):
+
+    contents = 'phone\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986\n+44 7700 900987\n+44 7700 900988\n+44 7700 900989'  # noqa
+
+    file_data = (BytesIO(contents.encode('utf-8')), 'valid.csv')
+    m_open = mock_open(read_data=contents)
+    _setup_mocker_for_nonemtpy_file(mocker)
 
     with notifications_admin.test_request_context():
         with notifications_admin.test_client() as client:
             user = create_test_user('active')
             client.login(user)
-            file_contents = 'phone\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986\n+44 7700 900987\n+44 7700 900988\n+44 7700 900989'.encode('utf-8')  # noqa
+            upload_data = {'file': file_data}
-
+            with mock.patch('app.main.views.sms.open', m_open):
-            upload_data = {'file': (BytesIO(file_contents), 'valid.csv')}
+                response = client.post('/services/123/sms/send',
-            response = client.post('/services/123/sms/send', data=upload_data,
+                                       data=upload_data,
-                                   follow_redirects=True)
+                                       follow_redirects=True)
 
         content = response.get_data(as_text=True)
 
@@ -76,18 +86,24 @@ def test_upload_csvfile_with_valid_phone_shows_first3_and_last3_numbers(
 
 
 def test_upload_csvfile_with_valid_phone_shows_all_if_6_or_less_numbers(
-        notifications_admin, notifications_admin_db, notify_db_session):
+        notifications_admin, notifications_admin_db, notify_db_session,
+        mocker):
+
+    contents = 'phone\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986'  # noqa
+
+    file_data = (BytesIO(contents.encode('utf-8')), 'valid.csv')
+    m_open = mock_open(read_data=contents)
+    _setup_mocker_for_nonemtpy_file(mocker)
 
     with notifications_admin.test_request_context():
         with notifications_admin.test_client() as client:
             user = create_test_user('active')
             client.login(user)
-
+            upload_data = {'file': file_data}
-            file_contents = 'phone\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986'.encode('utf-8')  # noqa
+            with mock.patch('app.main.views.sms.open', m_open):
-
+                response = client.post('/services/123/sms/send',
-            upload_data = {'file': (BytesIO(file_contents), 'valid.csv')}
+                                       data=upload_data,
-            response = client.post('/services/123/sms/send', data=upload_data,
+                                       follow_redirects=True)
-                                   follow_redirects=True)
 
         content = response.get_data(as_text=True)
 
@@ -103,13 +119,34 @@ def test_upload_csvfile_with_valid_phone_shows_all_if_6_or_less_numbers(
 
 
 def test_should_redirect_to_job(notifications_admin, notifications_admin_db,
-                                notify_db_session):
+                                notify_db_session, mocker):
+    _setup_mocker_for_check(mocker)
     with notifications_admin.test_request_context():
         with notifications_admin.test_client() as client:
             user = create_test_user('active')
             client.login(user)
+            with client.session_transaction() as s:
+                s[456] = 'test.csv'
 
-        response = client.post('/services/123/sms/check')
+        response = client.post('/services/123/sms/check',
+                               data={'recipients': 'test.csv'})
 
         assert response.status_code == 302
-        assert response.location == 'http://localhost/services/123/jobs/456'
+
+
+def _setup_mocker_for_empty_file(mocker):
+    mocker.patch('werkzeug.datastructures.FileStorage.save')
+    mocker.patch('os.remove')
+    ret = ValueError('The file emtpy.csv contained no data')
+    mocker.patch('app.main.views.sms._check_file', side_effect=ret)
+
+
+def _setup_mocker_for_nonemtpy_file(mocker):
+    mocker.patch('werkzeug.datastructures.FileStorage.save')
+    mocker.patch('os.remove')
+    mocker.patch('app.main.views.sms._check_file')
+
+
+def _setup_mocker_for_check(mocker):
+    mocker.patch('app.main.views.sms.s3upload').return_value = 456
+