From f0aebd00eb9c815a2e1e015046a4a8c7debd8189 Mon Sep 17 00:00:00 2001 From: Akita Noek Date: Tue, 19 Apr 2016 10:45:53 -0400 Subject: [PATCH] Removed transaction=True from pytests This was overkill and cost 3s per instance, all we needed to do is wrap things that expectedly fail transactions with transaction.atomic() --- awx/main/tests/functional/test_projects.py | 74 ++++++++++++++-------- awx/main/tests/functional/test_rbac_api.py | 17 +++-- 2 files changed, 56 insertions(+), 35 deletions(-) diff --git a/awx/main/tests/functional/test_projects.py b/awx/main/tests/functional/test_projects.py index e333f8be2b..62e98425df 100644 --- a/awx/main/tests/functional/test_projects.py +++ b/awx/main/tests/functional/test_projects.py @@ -1,16 +1,16 @@ import mock # noqa import pytest +from django.db import transaction from django.core.urlresolvers import reverse from awx.main.models import Project - # # Project listing and visibility tests # -@pytest.mark.django_db(transaction=True) +@pytest.mark.django_db def test_user_project_list(get, project_factory, organization, admin, alice, bob): 'List of projects a user has access to, filtered by projects you can also see' @@ -43,9 +43,7 @@ def test_user_project_list(get, project_factory, organization, admin, alice, bob assert get(reverse('api:user_projects_list', args=(admin.pk,)), alice).data['count'] == 2 -@pytest.mark.django_db(transaction=True) -def test_team_project_list(get, project_factory, team_factory, admin, alice, bob): - 'List of projects a team has access to, filtered by projects you can also see' +def setup_test_team_project_list(project_factory, team_factory, admin, alice, bob): team1 = team_factory('team1') team2 = team_factory('team2') @@ -61,6 +59,12 @@ def test_team_project_list(get, project_factory, team_factory, admin, alice, bob team1.member_role.members.add(alice) team2.member_role.members.add(bob) + return team1, team2 + +@pytest.mark.django_db +def test_team_project_list(get, project_factory, team_factory, admin, alice, bob): + 'List of projects a team has access to, filtered by projects you can also see' + team1, team2 = setup_test_team_project_list(project_factory, team_factory, admin, alice, bob) # admins can see all projects on a team assert get(reverse('api:team_projects_list', args=(team1.pk,)), admin).data['count'] == 2 @@ -69,17 +73,16 @@ def test_team_project_list(get, project_factory, team_factory, admin, alice, bob # users can see all projects on teams they are a member of assert get(reverse('api:team_projects_list', args=(team1.pk,)), alice).data['count'] == 2 - # alice should not be able to see team2 projects because she doesn't have access to team2 - res = get(reverse('api:team_projects_list', args=(team2.pk,)), alice) - assert res.status_code == 403 # but if she does, then she should only see the shared project team2.auditor_role.members.add(alice) assert get(reverse('api:team_projects_list', args=(team2.pk,)), alice).data['count'] == 1 team2.auditor_role.members.remove(alice) - # Test user endpoints first, very similar tests to test_user_project_list # but permissions are being derived from team membership instead. + with transaction.atomic(): + res = get(reverse('api:user_projects_list', args=(bob.pk,)), alice) + assert res.status_code == 403 # admins can see all projects assert get(reverse('api:user_projects_list', args=(admin.pk,)), admin).data['count'] == 3 @@ -91,28 +94,43 @@ def test_team_project_list(get, project_factory, team_factory, admin, alice, bob # users can see their own projects assert get(reverse('api:user_projects_list', args=(alice.pk,)), alice).data['count'] == 2 - # alice should not be able to see bob - res = get(reverse('api:user_projects_list', args=(bob.pk,)), alice) - assert res.status_code == 403 - # alice should see all projects they can see when viewing an admin assert get(reverse('api:user_projects_list', args=(admin.pk,)), alice).data['count'] == 2 +@pytest.mark.django_db +def test_team_project_list_fail1(get, project_factory, team_factory, admin, alice, bob): + # alice should not be able to see team2 projects because she doesn't have access to team2 + team1, team2 = setup_test_team_project_list(project_factory, team_factory, admin, alice, bob) + res = get(reverse('api:team_projects_list', args=(team2.pk,)), alice) + assert res.status_code == 403 +@pytest.mark.django_db +def test_team_project_list_fail2(get, project_factory, team_factory, admin, alice, bob): + team1, team2 = setup_test_team_project_list(project_factory, team_factory, admin, alice, bob) + # alice should not be able to see bob -@pytest.mark.django_db(transaction=True) -def test_create_project(post, organization, org_admin, org_member, admin, rando): - test_list = [rando, org_member, org_admin, admin] - expected_status_codes = [403, 403, 201, 201] +@pytest.mark.parametrize("u,expected_status_code", [ + ('rando', 403), + ('org_member', 403), + ('org_admin', 201), + ('admin', 201) +]) +@pytest.mark.django_db() +def test_create_project(post, organization, org_admin, org_member, admin, rando, u, expected_status_code): + if u == 'rando': + u = rando + elif u == 'org_member': + u = org_member + elif u == 'org_admin': + u = org_admin + elif u == 'admin': + u = admin - for i, u in enumerate(test_list): - result = post(reverse('api:project_list'), { - 'name': 'Project %d' % i, - 'organization': organization.id, - }, u) - print(result.data) - assert result.status_code == expected_status_codes[i] - if expected_status_codes[i] == 201: - assert Project.objects.filter(name='Project %d' % i, organization=organization).exists() - else: - assert not Project.objects.filter(name='Project %d' % i, organization=organization).exists() + result = post(reverse('api:project_list'), { + 'name': 'Project', + 'organization': organization.id, + }, u) + print(result.data) + assert result.status_code == expected_status_code + if expected_status_code == 201: + assert Project.objects.filter(name='Project', organization=organization).exists() diff --git a/awx/main/tests/functional/test_rbac_api.py b/awx/main/tests/functional/test_rbac_api.py index 1fc1485e14..cb75cd33ef 100644 --- a/awx/main/tests/functional/test_rbac_api.py +++ b/awx/main/tests/functional/test_rbac_api.py @@ -1,6 +1,7 @@ import mock # noqa import pytest +from django.db import transaction from django.core.urlresolvers import reverse from awx.main.models.rbac import Role, ROLE_SINGLETON_SYSTEM_ADMINISTRATOR @@ -266,7 +267,7 @@ def test_remove_user_to_role(post, admin, role): post(url, {'disassociate': True, 'id': admin.id}, admin) assert role.members.filter(id=admin.id).count() == 0 -@pytest.mark.django_db(transaction=True) +@pytest.mark.django_db def test_org_admin_add_user_to_job_template(post, organization, check_jobtemplate, user): 'Tests that a user with permissions to assign/revoke membership to a particular role can do so' org_admin = user('org-admin') @@ -280,8 +281,8 @@ def test_org_admin_add_user_to_job_template(post, organization, check_jobtemplat assert joe in check_jobtemplate.execute_role -@pytest.mark.django_db(transaction=True) -def test_org_admin_remove_user_to_job_template(post, organization, check_jobtemplate, user): +@pytest.mark.django_db +def test_org_admin_remove_user_from_job_template(post, organization, check_jobtemplate, user): 'Tests that a user with permissions to assign/revoke membership to a particular role can do so' org_admin = user('org-admin') joe = user('joe') @@ -295,7 +296,7 @@ def test_org_admin_remove_user_to_job_template(post, organization, check_jobtemp assert joe not in check_jobtemplate.execute_role -@pytest.mark.django_db(transaction=True) +@pytest.mark.django_db def test_user_fail_to_add_user_to_job_template(post, organization, check_jobtemplate, user): 'Tests that a user without permissions to assign/revoke membership to a particular role cannot do so' rando = user('rando') @@ -304,13 +305,14 @@ def test_user_fail_to_add_user_to_job_template(post, organization, check_jobtemp assert rando not in check_jobtemplate.admin_role assert joe not in check_jobtemplate.execute_role - res = post(reverse('api:role_users_list', args=(check_jobtemplate.execute_role.id,)), {'id': joe.id}, rando) + with transaction.atomic(): + res = post(reverse('api:role_users_list', args=(check_jobtemplate.execute_role.id,)), {'id': joe.id}, rando) assert res.status_code == 403 assert joe not in check_jobtemplate.execute_role -@pytest.mark.django_db(transaction=True) +@pytest.mark.django_db def test_user_fail_to_remove_user_to_job_template(post, organization, check_jobtemplate, user): 'Tests that a user without permissions to assign/revoke membership to a particular role cannot do so' rando = user('rando') @@ -320,7 +322,8 @@ def test_user_fail_to_remove_user_to_job_template(post, organization, check_jobt assert rando not in check_jobtemplate.admin_role assert joe in check_jobtemplate.execute_role - res = post(reverse('api:role_users_list', args=(check_jobtemplate.execute_role.id,)), {'disassociate': True, 'id': joe.id}, rando) + with transaction.atomic(): + res = post(reverse('api:role_users_list', args=(check_jobtemplate.execute_role.id,)), {'disassociate': True, 'id': joe.id}, rando) assert res.status_code == 403 assert joe in check_jobtemplate.execute_role