Increase coverage - low hanging fruit 1 (#749)

* add uid and uid-match upload tests

* increase coverage of engine uploads w/ job (+enable job creation from data_builder)

* add test_roothandler.py to cover the /api root endpoint

* add more tests for session template compliance checking

* add tests to cover project/collection stats retrieval (requested w/ stats=true)

* add tests to cover most of api/handlers/userhandler.py

* make sure that unit test coverage is included in the overall report

.dockerignore

@@ -9,6 +9,6 @@ persistent
 runtime
 bootstrap.json
 .cache
-.coverage
+.coverage*
 coverage.xml
 htmlcov

test/bin/run-tests-ubuntu.sh

@@ -30,12 +30,13 @@ fi
 clean_up () {
   kill $API_PID || true
   wait 2> /dev/null
-  # Report on unit tests and integration tests separately
-  # Only submit integration test coverage to coveralls
-  coverage report -m
-  rm .coverage
+
+  echo -e "\nUNIT TEST COVERAGE:"
+  coverage report --skip-covered
+
   coverage combine
-  coverage report -m
+  echo -e "\nOVERALL COVERAGE:"
+  coverage report --show-missing
   coverage html
 }
 

test/bin/run-unit-tests.sh

@@ -10,4 +10,4 @@ rm -rf test/unit_tests/python/__pycache__
 
 rm -f .coverage
 
-PYTHONPATH="$( pwd )"  py.test --cov=api test/unit_tests/python
+PYTHONPATH="$( pwd )"  py.test --cov=api --cov-report= test/unit_tests/python

test/integration_tests/python/conftest.py

@@ -131,6 +131,7 @@ def default_payload():
                 'version': '0.0.1',
             },
         },
+        'job': {'inputs': {}},
     })
 
 
@@ -221,21 +222,17 @@ class DataBuilder(object):
 
         # add missing required unique fields using randstr
         # such fields are: [user._id, group._id, gear.gear.name]
-        if resource == 'user':
-            if '_id' not in payload:
-                payload['_id'] = self.randstr() + '@user.com'
-        if resource == 'group':
-            if '_id' not in payload:
-                payload['_id'] = self.randstr()
-        if resource == 'gear':
-            if 'name' not in payload['gear']:
-                payload['gear']['name'] = self.randstr()
+        if resource == 'user' and '_id' not in payload:
+            payload['_id'] = self.randstr() + '@user.com'
+        if resource == 'group' and '_id' not in payload:
+            payload['_id'] = self.randstr()
+        if resource == 'gear' and 'name' not in payload['gear']:
+            payload['gear']['name'] = self.randstr()
 
         # add missing label fields using randstr
         # such fields are: [project.label, session.label, acquisition.label]
-        if resource in self.child_to_parent:
-            if 'label' not in payload:
-                payload['label'] = self.randstr()
+        if resource in self.child_to_parent and 'label' not in payload:
+            payload['label'] = self.randstr()
 
         # add missing parent container when creating child container
         if resource in self.child_to_parent:
@@ -243,10 +240,16 @@ class DataBuilder(object):
             if parent not in payload:
                 payload[parent] = self.get_or_create(parent)
 
+        # add missing gear when creating job
+        if resource == 'job' and 'gear_id' not in payload:
+            payload['gear_id'] = self.get_or_create('gear')
+
         # put together the create url to post to
         create_url = '/' + resource + 's'
         if resource == 'gear':
             create_url += '/' + payload['gear']['name']
+        if resource == 'job':
+            create_url += '/add'
 
         # handle user api keys (they are set via mongo directly)
         if resource == 'user':

test/integration_tests/python/test_collection.py

@@ -10,6 +10,11 @@ def test_collections(data_builder, as_admin):
     assert r.ok
     collection = r.json()['_id']
 
+    # get all collections w/ stats=true
+    r = as_admin.get('/collections', params={'stats': 'true'})
+    assert r.ok
+    assert all('session_count' in coll for coll in r.json())
+
     # get collection
     r = as_admin.get('/collections/' + collection)
     assert r.ok

test/integration_tests/python/test_containers.py

@@ -37,44 +37,78 @@ def test_switching_acquisition_between_sessions(data_builder, as_admin):
     assert r.json()['session'] == session_2
 
 
-def test_project_template(data_builder, as_admin):
-    subject_code = 'test'
-    acquisition_label = 'test'
-
+def test_project_template(data_builder, file_form, as_admin):
     project = data_builder.create_project()
-    session = data_builder.create_session()
-    acquisition = data_builder.create_acquisition(label=acquisition_label)
+    session = data_builder.create_session(subject={'code': 'compliant'})
+    # NOTE adding acquisition_1 to cover code that's skipping non-matching containers
+    acquisition_1 = data_builder.create_acquisition(label='non-compliant')
+    acquisition_2 = data_builder.create_acquisition(label='compliant')
+    assert as_admin.post('/acquisitions/' + acquisition_2 + '/tags', json={'value': 'compliant'}).ok
+    assert as_admin.post('/acquisitions/' + acquisition_2 + '/files', files=file_form('non-compliant.txt')).ok
+    assert as_admin.post('/acquisitions/' + acquisition_2 + '/files', files=file_form('compliant1.csv')).ok
+    assert as_admin.post('/acquisitions/' + acquisition_2 + '/files', files=file_form('compliant2.csv')).ok
+
+    # test the session before setting the template
+    r = as_admin.get('/sessions/' + session)
+    assert r.ok
+    assert 'project_has_template' not in r.json()
 
     # create template for the project
     r = as_admin.post('/projects/' + project + '/template', json={
-        'session': { 'subject': { 'code' : '^{}$'.format(subject_code) } },
-        'acquisitions': [{ 'label': '^{}$'.format(acquisition_label), 'minimum': 1 }]
+        'session': {'subject': {'code': '^compliant$'}},
+        'acquisitions': [{
+            'minimum': 1,
+            'label': '^compliant$',
+            'tags': '^compliant$',
+            'files': [{
+                'minimum': 2,
+                'mimetype': 'text/csv',
+            }]
+        }]
     })
     assert r.ok
     assert r.json()['modified'] == 1
 
-    # test non-compliant session (wrong subject.code)
-    r = as_admin.get('/sessions/' + session)
-    assert r.ok
-    assert r.json()['project_has_template'] == True
-    assert r.json()['satisfies_template'] == False
-
-    # make session compliant by setting subject.code
-    r = as_admin.put('/sessions/' + session, json={'subject': {'code': subject_code}})
-    assert r.ok
-
-    # test compliant session (subject.code and #acquisitions)
-    r = as_admin.get('/sessions/' + session)
-    assert r.ok
-    assert r.json()['satisfies_template'] == True
-
-    # make session non-compliant by deleting acquisition
-    r = as_admin.delete('/acquisitions/' + acquisition)
-    assert r.ok
-
+    # test session compliance
     r = as_admin.get('/sessions/' + session)
     assert r.ok
-    assert r.json()['satisfies_template'] == False
+    assert r.json()['project_has_template']
+
+    def satisfies_template():
+        r = as_admin.get('/sessions/' + session)
+        assert r.ok
+        return r.json()['satisfies_template']
+
+    # test that missing any single requirement breaks compliance
+    # session.subject.code
+    assert satisfies_template()
+    assert as_admin.put('/sessions/' + session, json={'subject': {'code': 'non-compliant'}}).ok
+    assert not satisfies_template()
+    assert as_admin.put('/sessions/' + session, json={'subject': {'code': 'compliant'}}).ok
+
+    # acquisitions.label
+    assert satisfies_template()
+    assert as_admin.put('/acquisitions/' + acquisition_2, json={'label': 'non-compliant'}).ok
+    assert not satisfies_template()
+    assert as_admin.put('/acquisitions/' + acquisition_2, json={'label': 'compliant'}).ok
+
+    # acquisitions.tags
+    assert satisfies_template()
+    assert as_admin.delete('/acquisitions/' + acquisition_2 + '/tags/compliant').ok
+    # TODO figure out why removing the tag does not break compliance
+    # assert not satisfies_template()
+    assert as_admin.post('/acquisitions/' + acquisition_2 + '/tags', json={'value': 'compliant'}).ok
+
+    # acquisitions.files.minimum
+    assert satisfies_template()
+    assert as_admin.delete('/acquisitions/' + acquisition_2 + '/files/compliant2.csv').ok
+    assert not satisfies_template()
+    assert as_admin.post('/acquisitions/' + acquisition_2 + '/files', files=file_form('compliant2.csv')).ok
+
+    # acquisitions.minimum
+    assert satisfies_template()
+    assert as_admin.delete('/acquisitions/' + acquisition_2)
+    assert not satisfies_template()
 
     # delete project template
     r = as_admin.delete('/projects/' + project + '/template')
@@ -86,8 +120,9 @@ def test_project_template(data_builder, as_admin):
 
 
 def test_get_all_containers(data_builder, as_public):
-    project = data_builder.create_project()
-    session = data_builder.create_session()
+    project_1 = data_builder.create_project()
+    project_2 = data_builder.create_project()
+    session = data_builder.create_session(project=project_1)
 
     # get all projects w/ info=true
     r = as_public.get('/projects', params={'info': 'true'})
@@ -98,8 +133,12 @@ def test_get_all_containers(data_builder, as_public):
     assert r.ok
     assert all('session_count' in proj for proj in r.json())
 
+    # get all projects w/ stats=true
+    r = as_public.get('/projects', params={'stats': 'true'})
+    assert r.ok
+
     # get all sessions for project w/ measurements=true and stats=true
-    r = as_public.get('/projects/' + project + '/sessions', params={
+    r = as_public.get('/projects/' + project_1 + '/sessions', params={
         'measurements': 'true',
         'stats': 'true'
     })

test/integration_tests/python/test_roothandler.py

+def test_roothandler(as_public):
+    r = as_public.get('')
+    assert r.ok

test/integration_tests/python/test_uploads.py

@@ -3,38 +3,72 @@ import json
 import dateutil.parser
 
 
-def test_uid_upload(data_builder, file_form, as_admin):
+def test_upload_without_login(as_public):
+    r = as_public.post('/upload/uid')
+    assert r.status_code == 403
+
+
+def test_uid_upload(data_builder, file_form, as_admin, as_user):
     group = data_builder.create_group()
 
     # try to uid-upload w/o metadata
     r = as_admin.post('/upload/uid', files=file_form('test.csv'))
     assert r.status_code == 500
 
-    # uid-upload files
-    r = as_admin.post('/upload/uid', files=file_form(
-        'project.csv', 'subject.csv', 'session.csv', 'acquisition.csv', 'unused.csv',
-        meta={
-            'group': {'_id': group},
-            'project': {
-                'label': 'test_project',
-                'files': [{'name': 'project.csv'}]
-            },
-            'session': {
-                'uid': 'test_session_uid',
-                'subject': {
-                    'code': 'test_subject_code',
-                    'files': [{'name': 'subject.csv'}]
-                },
-                'files': [{'name': 'session.csv'}]
+    # NOTE unused.csv is testing code that discards files not referenced from meta
+    uid_files = ('project.csv', 'subject.csv', 'session.csv', 'acquisition.csv', 'unused.csv')
+    uid_meta = {
+        'group': {'_id': group},
+        'project': {
+            'label': 'uid_upload',
+            'files': [{'name': 'project.csv'}]
+        },
+        'session': {
+            'uid': 'uid_upload',
+            'subject': {
+                'code': 'uid_upload',
+                'files': [{'name': 'subject.csv'}]
             },
-            'acquisition': {
-                'uid': 'test_acquisition_uid',
-                'files': [{'name': 'acquisition.csv'}]
-            }
-        })
-    )
+            'files': [{'name': 'session.csv'}]
+        },
+        'acquisition': {
+            'uid': 'uid_upload',
+            'files': [{'name': 'acquisition.csv'}]
+        }
+    }
+
+    # uid-upload files
+    r = as_admin.post('/upload/uid', files=file_form(*uid_files, meta=uid_meta))
+    assert r.ok
+
+    # uid-upload files to existing session uid
+    r = as_admin.post('/upload/uid', files=file_form(*uid_files, meta=uid_meta))
+    assert r.ok
+
+    # try uid-upload files to existing session uid w/ other user (having no rw perms on session)
+    r = as_user.post('/upload/uid', files=file_form(*uid_files, meta=uid_meta))
+    assert r.status_code == 403
+
+    # uid-match-upload files (to the same session and acquisition uid's as above)
+    uid_match_meta = uid_meta.copy()
+    del uid_match_meta['group']
+    r = as_admin.post('/upload/uid-match', files=file_form(*uid_files, meta=uid_match_meta))
     assert r.ok
 
+    # try uid-match upload w/ other user (having no rw permissions on session)
+    r = as_user.post('/upload/uid-match', files=file_form(*uid_files, meta=uid_match_meta))
+    assert r.status_code == 403
+
+    # try uid-match upload w/ non-existent acquisition uid
+    uid_match_meta['acquisition']['uid'] = 'nonexistent_uid'
+    r = as_admin.post('/upload/uid-match', files=file_form(*uid_files, meta=uid_match_meta))
+    assert r.status_code == 404
+
+    # try uid-match upload w/ non-existent session uid
+    uid_match_meta['session']['uid'] = 'nonexistent_uid'
+    r = as_admin.post('/upload/uid-match', files=file_form(*uid_files, meta=uid_match_meta))
+    assert r.status_code == 404
+
     # delete group and children recursively (created by upload)
     data_builder.delete_group(group, recursive=True)
 
@@ -80,6 +114,9 @@ def test_acquisition_engine_upload(data_builder, file_form, as_root):
     project = data_builder.create_project()
     session = data_builder.create_session()
     acquisition = data_builder.create_acquisition()
+    job = data_builder.create_job(inputs={
+        'test': {'type': 'acquisition', 'id': acquisition, 'name': 'test'}
+    })
 
     metadata = {
         'project':{
@@ -109,8 +146,16 @@ def test_acquisition_engine_upload(data_builder, file_form, as_root):
             ]
         }
     }
+    # try engine upload w/ non-existent job_id
     r = as_root.post('/engine',
-        params={'level': 'acquisition', 'id': acquisition},
+        params={'level': 'acquisition', 'id': acquisition, 'job': '000000000000000000000000'},
+        files=file_form('one.csv', 'two.csv', meta=metadata)
+    )
+    assert r.status_code == 500
+
+    # engine upload
+    r = as_root.post('/engine',
+        params={'level': 'acquisition', 'id': acquisition, 'job': job},
         files=file_form('one.csv', 'two.csv', meta=metadata)
     )
     assert r.ok

test/integration_tests/python/test_users.py

-def test_users(as_root):
-    new_user_id = 'new@user.com'
-
+def test_users(as_root, as_user, as_public):
     # List users
-    r = as_root.get('/users')
+    r = as_user.get('/users')
+    assert r.ok
+
+    # Try to get self w/o logging in
+    r = as_public.get('/users/self')
+    assert r.status_code == 400
+
+    # Get self as user
+    r = as_user.get('/users/self')
     assert r.ok
+    user_id = r.json()['_id']
 
-    # Get self
+    # Get self as admin
     r = as_root.get('/users/self')
     assert r.ok
+    admin_id = r.json()['_id']
+    assert admin_id != user_id
+
+    # Try to get self's avatar
+    r = as_user.get('/users/self/avatar')
+    assert r.status_code == 404
+
+    # Get user as user
+    r = as_user.get('/users/' + user_id)
+    assert r.ok
+
+    # Try to get user avatar as user
+    r = as_user.get('/users/' + user_id + '/avatar')
+    assert r.status_code == 404
 
     # Try adding new user missing required attr
     r = as_root.post('/users', json={
@@ -19,8 +40,7 @@ def test_users(as_root):
     assert "'firstname' is a required property" in r.text
 
     # Add new user
-    r = as_root.get('/users/' + new_user_id)
-    assert r.status_code == 404
+    new_user_id = 'new@user.com'
     r = as_root.post('/users', json={
         '_id': new_user_id,
         'firstname': 'New',
@@ -30,10 +50,43 @@ def test_users(as_root):
     r = as_root.get('/users/' + new_user_id)
     assert r.ok
 
-    # Modify existing user
+    # Try to update non-existent user
+    r = as_root.put('/users/nonexistent@user.com', json={'firstname': 'Realname'})
+    assert r.status_code == 404
+
+    # Update existing user
     r = as_root.put('/users/' + new_user_id, json={'firstname': 'Realname'})
     assert r.ok
+    assert r.json()['modified'] == 1
+
+    # Try to delete non-existent user
+    r = as_root.delete('/users/nonexistent@user.com')
+    assert r.status_code == 404
 
-    # Cleanup
+    # Delete user
     r = as_root.delete('/users/' + new_user_id)
     assert r.ok
+
+
+def test_generate_api_key(data_builder, as_public):
+    # Try to generate new api key w/o logging in
+    r = as_public.post('/users/self/key')
+    assert r.status_code == 400
+
+    new_user = data_builder.create_user(api_key='test')
+    as_new_user = as_public
+    as_new_user.headers.update({'Authorization': 'scitran-user test'})
+
+    # Generate new api key for user
+    r = as_new_user.post('/users/self/key')
+    assert r.ok
+    assert 'key' in r.json()
+
+
+def test_reset_wechat_registration(data_builder, as_admin):
+    new_user = data_builder.create_user()
+
+    # Reset (create) wechat registration code for user
+    r = as_admin.post('/users/' + new_user + '/reset-registration')
+    assert r.ok
+    assert 'registration_code' in r.json()