Replace instances of client.login with client_request

We have a `client_request` fixture which does a bunch of useful stuff
like:
- checking the status code of the response
- returning a `BeautifulSoup` object

Lots of our tests still use an older fixture called `client`. This is
not as good because it:
- returns a raw `Response` object
- doesn’t do the additional checks
- means our tests contain a lot of repetetive boilerplate like `page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')`

This commit converts all the tests which had a `client.login(…)`
statement to use `client_request` (which is already logged in by
default).

Subsequent commits will remove uses of `client` in other tests, but
doing it this way means the work can be broken up into more manageable
chunks.

app/templates/views/providers/edit-provider.html

@@ -5,7 +5,7 @@
 {% from "components/back-link/macro.njk" import govukBackLink %}
 
 {% block per_page_title %}
-Provider - {{provider.display_name}}
+  {{provider.display_name}}
 {% endblock %}
 
 {% block backLink %}

app/templates/views/providers/provider.html

@@ -5,7 +5,7 @@
 {% from "components/back-link/macro.njk" import govukBackLink %}
 
 {% block per_page_title %}
-Provider versions
+  {{ provider_versions[0].display_name }}
 {% endblock %}
 
 {% block backLink %}

app/templates/views/providers/providers.html

@@ -70,7 +70,7 @@ Providers
 
   {% endcall %}
 
-  <h1 class="heading-medium">International SMS Providers</h1>
+  <h2 class="heading-medium">International SMS Providers</h2>
 
   {% call(item, row_number) list_table(
       intl_sms_providers,

tests/app/main/views/accounts/test_show_accounts_or_dashboard.py

@@ -27,130 +27,128 @@ def user_with_orgs_and_services(num_orgs, num_services, platform_admin=False):
     (1, 0, '.organisation_dashboard', {'org_id': 'org1'}),
 ])
 def test_show_accounts_or_dashboard_redirects_to_choose_account_or_service_dashboard(
-    client,
-    mocker,
+    client_request,
     mock_get_organisations_and_services_for_user,
     num_orgs,
     num_services,
     endpoint,
     endpoint_kwargs
 ):
-    client.login(user_with_orgs_and_services(num_orgs=num_orgs, num_services=num_services), mocker=mocker)
+    client_request.login(user_with_orgs_and_services(num_orgs=num_orgs, num_services=num_services))
 
-    response = client.get(url_for('main.show_accounts_or_dashboard'))
-
-    assert response.status_code == 302
-    assert response.location == url_for(endpoint, _external=True, **endpoint_kwargs)
-
-
-def test_show_accounts_or_dashboard_redirects_if_service_in_session(client, mocker, mock_get_service):
-    client.login(user_with_orgs_and_services(num_orgs=1, num_services=1), mocker=mocker)
-    with client.session_transaction() as session:
-        session['service_id'] = 'service1'
-        session['organisation_id'] = None
-
-    response = client.get(url_for('.show_accounts_or_dashboard'))
-
-    assert response.status_code == 302
-    assert response.location == url_for(
-        'main.service_dashboard',
-        service_id='service1',
-        _external=True
+    client_request.get(
+        'main.show_accounts_or_dashboard',
+        _expected_redirect=url_for(endpoint, _external=True, **endpoint_kwargs)
     )
 
 
-def test_show_accounts_or_dashboard_redirects_if_org_in_session(client, mocker):
-    client.login(user_with_orgs_and_services(num_orgs=1, num_services=1), mocker=mocker)
-    with client.session_transaction() as session:
+def test_show_accounts_or_dashboard_redirects_if_service_in_session(client_request, mock_get_service):
+    client_request.login(user_with_orgs_and_services(num_orgs=1, num_services=1))
+    with client_request.session_transaction() as session:
+        session['service_id'] = 'service1'
+        session['organisation_id'] = None
+
+    client_request.get(
+        '.show_accounts_or_dashboard',
+        _expected_redirect=url_for(
+            'main.service_dashboard',
+            service_id='service1',
+            _external=True
+        ),
+    )
+
+
+def test_show_accounts_or_dashboard_redirects_if_org_in_session(client_request):
+    client_request.login(user_with_orgs_and_services(num_orgs=1, num_services=1))
+    with client_request.session_transaction() as session:
         session['service_id'] = None
         session['organisation_id'] = 'org1'
 
-    response = client.get(url_for('.show_accounts_or_dashboard'))
-
-    assert response.status_code == 302
-    assert response.location == url_for(
-        'main.organisation_dashboard',
-        org_id='org1',
-        _external=True
+    client_request.get(
+        '.show_accounts_or_dashboard',
+        _expected_redirect=url_for(
+            'main.organisation_dashboard',
+            org_id='org1',
+            _external=True
+        ),
     )
 
 
 def test_show_accounts_or_dashboard_doesnt_redirect_to_service_dashboard_if_user_not_part_of_service_in_session(
-    client,
-    mocker,
+    client_request,
     mock_get_organisations_and_services_for_user,
     mock_get_service
 ):
-    client.login(user_with_orgs_and_services(num_orgs=1, num_services=1), mocker=mocker)
-    with client.session_transaction() as session:
+    client_request.login(user_with_orgs_and_services(num_orgs=1, num_services=1))
+    with client_request.session_transaction() as session:
         session['service_id'] = 'service2'
         session['organisation_id'] = None
 
-    response = client.get(url_for('.show_accounts_or_dashboard'))
-
-    assert response.status_code == 302
-    assert response.location == url_for('main.organisation_dashboard', org_id='org1', _external=True)
+    client_request.get(
+        '.show_accounts_or_dashboard',
+        _expected_redirect=url_for('main.organisation_dashboard', org_id='org1', _external=True)
+    )
 
 
 def test_show_accounts_or_dashboard_doesnt_redirect_to_org_dashboard_if_user_not_part_of_org_in_session(
-    client,
-    mocker,
+    client_request,
     mock_get_organisations_and_services_for_user,
 ):
-    client.login(user_with_orgs_and_services(num_orgs=1, num_services=1), mocker=mocker)
-    with client.session_transaction() as session:
+    client_request.login(user_with_orgs_and_services(num_orgs=1, num_services=1))
+    with client_request.session_transaction() as session:
         session['service_id'] = None
         session['organisation_id'] = 'org2'
 
-    response = client.get(url_for('.show_accounts_or_dashboard'))
-
-    assert response.status_code == 302
-    assert response.location == url_for('main.organisation_dashboard', org_id='org1', _external=True)
+    client_request.get(
+        '.show_accounts_or_dashboard',
+        _expected_redirect=url_for('main.organisation_dashboard', org_id='org1', _external=True)
+    )
 
 
 def test_show_accounts_or_dashboard_redirects_if_not_logged_in(
-    client,
+    client_request,
     notify_admin,
 ):
-    response = client.get(url_for('main.show_accounts_or_dashboard'))
-    assert response.status_code == 302
-    assert response.location == url_for('main.index', _external=True)
+    client_request.logout()
+    client_request.get(
+        'main.show_accounts_or_dashboard',
+        _expected_redirect=url_for('main.index', _external=True),
+    )
 
 
 def test_show_accounts_or_dashboard_redirects_to_service_dashboard_if_platform_admin(
-    client,
+    client_request,
     mocker,
     mock_get_service
 ):
-    client.login(user_with_orgs_and_services(num_orgs=1, num_services=1, platform_admin=True), mocker=mocker)
-    with client.session_transaction() as session:
+    client_request.login(user_with_orgs_and_services(num_orgs=1, num_services=1, platform_admin=True))
+    with client_request.session_transaction() as session:
         session['service_id'] = 'service2'
         session['organisation_id'] = None
 
-    response = client.get(url_for('.show_accounts_or_dashboard'))
-
-    assert response.status_code == 302
-    assert response.location == url_for(
-        'main.service_dashboard',
-        service_id='service2',
-        _external=True
+    client_request.get(
+        '.show_accounts_or_dashboard',
+        _expected_redirect=url_for(
+            'main.service_dashboard',
+            service_id='service2',
+            _external=True
+        ),
     )
 
 
 def test_show_accounts_or_dashboard_redirects_to_org_dashboard_if_platform_admin(
-    client,
-    mocker
+    client_request,
 ):
-    client.login(user_with_orgs_and_services(num_orgs=1, num_services=1, platform_admin=True), mocker=mocker)
-    with client.session_transaction() as session:
+    client_request.login(user_with_orgs_and_services(num_orgs=1, num_services=1, platform_admin=True))
+    with client_request.session_transaction() as session:
         session['service_id'] = None
         session['organisation_id'] = 'org2'
 
-    response = client.get(url_for('.show_accounts_or_dashboard'))
-
-    assert response.status_code == 302
-    assert response.location == url_for(
-        'main.organisation_dashboard',
-        org_id='org2',
-        _external=True
+    client_request.get(
+        '.show_accounts_or_dashboard',
+        _expected_redirect=url_for(
+            'main.organisation_dashboard',
+            org_id='org2',
+            _external=True
+        ),
     )

tests/app/main/views/test_add_service.py

@@ -9,17 +9,16 @@ from tests.conftest import normalize_spaces
 
 
 def test_non_gov_user_cannot_see_add_service_button(
-    client,
+    client_request,
     mock_login,
     mock_get_non_govuser,
     api_nongov_user_active,
     mock_get_organisations,
     mock_get_organisations_and_services_for_user,
 ):
-    client.login(api_nongov_user_active)
-    response = client.get(url_for('main.choose_account'))
-    assert 'Add a new service' not in response.get_data(as_text=True)
-    assert response.status_code == 200
+    client_request.login(api_nongov_user_active)
+    page = client_request.get('main.choose_account')
+    assert 'Add a new service' not in page.text
 
 
 @pytest.mark.parametrize('org_json', (

tests/app/main/views/test_api_integration.py

@@ -171,21 +171,19 @@ def test_api_documentation_page_should_redirect(
 
 
 def test_should_show_empty_api_keys_page(
-    client,
+    client_request,
     api_user_active,
     mock_login,
     mock_get_no_api_keys,
     mock_get_service,
     mock_has_permissions,
 ):
-    client.login(api_user_active)
-    service_id = str(uuid.uuid4())
-    response = client.get(url_for('main.api_keys', service_id=service_id))
+    client_request.login(api_user_active)
+    page = client_request.get('main.api_keys', service_id=SERVICE_ONE_ID)
 
-    assert response.status_code == 200
-    assert 'You have not created any API keys yet' in response.get_data(as_text=True)
-    assert 'Create an API key' in response.get_data(as_text=True)
-    mock_get_no_api_keys.assert_called_once_with(service_id)
+    assert 'You have not created any API keys yet' in page.text
+    assert 'Create an API key' in page.text
+    mock_get_no_api_keys.assert_called_once_with(SERVICE_ONE_ID)
 
 
 def test_should_show_api_keys_page(

tests/app/main/views/test_dashboard.py

@@ -3,7 +3,6 @@ import json
 from datetime import datetime
 
 import pytest
-from bs4 import BeautifulSoup
 from flask import url_for
 from freezegun import freeze_time
 
@@ -1183,21 +1182,20 @@ def test_future_usage_page(
     mock_get_free_sms_fragment_limit.assert_called_with(SERVICE_ONE_ID, 2014)
 
 
-def _test_dashboard_menu(mocker, notify_admin, usr, service, permissions):
-    with notify_admin.test_request_context():
-        with notify_admin.test_client() as client:
-            usr['permissions'][str(service['id'])] = permissions
-            usr['services'] = [service['id']]
-            mocker.patch('app.user_api_client.check_verify_code', return_value=(True, ''))
-            mocker.patch('app.service_api_client.get_services', return_value={'data': [service]})
-            mocker.patch('app.user_api_client.get_user', return_value=usr)
-            mocker.patch('app.user_api_client.get_user_by_email', return_value=usr)
-            mocker.patch('app.service_api_client.get_service', return_value={'data': service})
-            client.login(usr)
-            return client.get(url_for('main.service_dashboard', service_id=service['id']))
+def _test_dashboard_menu(client_request, mocker, usr, service, permissions):
+    usr['permissions'][str(service['id'])] = permissions
+    usr['services'] = [service['id']]
+    mocker.patch('app.user_api_client.check_verify_code', return_value=(True, ''))
+    mocker.patch('app.service_api_client.get_services', return_value={'data': [service]})
+    mocker.patch('app.user_api_client.get_user', return_value=usr)
+    mocker.patch('app.user_api_client.get_user_by_email', return_value=usr)
+    mocker.patch('app.service_api_client.get_service', return_value={'data': service})
+    client_request.login(usr)
+    return client_request.get('main.service_dashboard', service_id=service['id'])
 
 
 def test_menu_send_messages(
+    client_request,
     mocker,
     notify_admin,
     api_user_active,
@@ -1213,29 +1211,29 @@ def test_menu_send_messages(
 ):
     service_one['permissions'] = ['email', 'sms', 'letter', 'upload_letters']
 
-    with notify_admin.test_request_context():
-        resp = _test_dashboard_menu(
-            mocker,
-            notify_admin,
-            api_user_active,
-            service_one,
-            ['view_activity', 'send_texts', 'send_emails', 'send_letters'])
-        page = resp.get_data(as_text=True)
-        assert url_for(
-            'main.choose_template',
-            service_id=service_one['id'],
-        ) in page
-        assert url_for('main.uploads', service_id=service_one['id']) in page
-        assert url_for('main.manage_users', service_id=service_one['id']) in page
+    page = _test_dashboard_menu(
+        client_request,
+        mocker,
+        api_user_active,
+        service_one,
+        ['view_activity', 'send_texts', 'send_emails', 'send_letters']
+    )
+    page = str(page)
+    assert url_for(
+        'main.choose_template',
+        service_id=service_one['id'],
+    ) in page
+    assert url_for('main.uploads', service_id=service_one['id']) in page
+    assert url_for('main.manage_users', service_id=service_one['id']) in page
 
-        assert url_for('main.service_settings', service_id=service_one['id']) not in page
-        assert url_for('main.api_keys', service_id=service_one['id']) not in page
-        assert url_for('main.view_providers') not in page
+    assert url_for('main.service_settings', service_id=service_one['id']) not in page
+    assert url_for('main.api_keys', service_id=service_one['id']) not in page
+    assert url_for('main.view_providers') not in page
 
 
 def test_menu_send_messages_when_service_does_not_have_upload_letters_permission(
+    client_request,
     mocker,
-    notify_admin,
     api_user_active,
     service_one,
     mock_get_service_templates,
@@ -1247,21 +1245,20 @@ def test_menu_send_messages_when_service_does_not_have_upload_letters_permission
     mock_get_free_sms_fragment_limit,
     mock_get_returned_letter_statistics_with_no_returned_letters,
 ):
-    with notify_admin.test_request_context():
-        resp = _test_dashboard_menu(
-            mocker,
-            notify_admin,
-            api_user_active,
-            service_one,
-            ['view_activity', 'send_texts', 'send_emails', 'send_letters'])
-        page = BeautifulSoup(resp.data.decode('utf-8'), 'html.parser')
-        assert page.select_one('.navigation')
-        assert url_for('main.uploads', service_id=service_one['id']) not in page.select_one('.navigation')
+    page = _test_dashboard_menu(
+        client_request,
+        mocker,
+        api_user_active,
+        service_one,
+        ['view_activity', 'send_texts', 'send_emails', 'send_letters'])
+
+    assert page.select_one('.navigation')
+    assert url_for('main.uploads', service_id=service_one['id']) not in page.select_one('.navigation')
 
 
 def test_menu_manage_service(
+    client_request,
     mocker,
-    notify_admin,
     api_user_active,
     service_one,
     mock_get_service_templates,
@@ -1273,27 +1270,26 @@ def test_menu_manage_service(
     mock_get_returned_letter_statistics_with_no_returned_letters,
     mock_get_free_sms_fragment_limit,
 ):
-    with notify_admin.test_request_context():
-        resp = _test_dashboard_menu(
-            mocker,
-            notify_admin,
-            api_user_active,
-            service_one,
-            ['view_activity', 'manage_templates', 'manage_users', 'manage_settings'])
-        page = resp.get_data(as_text=True)
-        assert url_for(
-            'main.choose_template',
-            service_id=service_one['id'],
-        ) in page
-        assert url_for('main.manage_users', service_id=service_one['id']) in page
-        assert url_for('main.service_settings', service_id=service_one['id']) in page
+    page = _test_dashboard_menu(
+        client_request,
+        mocker,
+        api_user_active,
+        service_one,
+        ['view_activity', 'manage_templates', 'manage_users', 'manage_settings'])
+    page = str(page)
+    assert url_for(
+        'main.choose_template',
+        service_id=service_one['id'],
+    ) in page
+    assert url_for('main.manage_users', service_id=service_one['id']) in page
+    assert url_for('main.service_settings', service_id=service_one['id']) in page
 
-        assert url_for('main.api_keys', service_id=service_one['id']) not in page
+    assert url_for('main.api_keys', service_id=service_one['id']) not in page
 
 
 def test_menu_manage_api_keys(
+    client_request,
     mocker,
-    notify_admin,
     api_user_active,
     service_one,
     mock_get_service_templates,
@@ -1305,25 +1301,24 @@ def test_menu_manage_api_keys(
     mock_get_returned_letter_statistics_with_no_returned_letters,
     mock_get_free_sms_fragment_limit,
 ):
-    with notify_admin.test_request_context():
-        resp = _test_dashboard_menu(
-            mocker,
-            notify_admin,
-            api_user_active,
-            service_one,
-            ['view_activity', 'manage_api_keys'])
+    page = _test_dashboard_menu(
+        client_request,
+        mocker,
+        api_user_active,
+        service_one,
+        ['view_activity', 'manage_api_keys'])
 
-        page = resp.get_data(as_text=True)
+    page = str(page)
 
-        assert url_for('main.choose_template', service_id=service_one['id'],) in page
-        assert url_for('main.manage_users', service_id=service_one['id']) in page
-        assert url_for('main.service_settings', service_id=service_one['id']) in page
-        assert url_for('main.api_integration', service_id=service_one['id']) in page
+    assert url_for('main.choose_template', service_id=service_one['id'],) in page
+    assert url_for('main.manage_users', service_id=service_one['id']) in page
+    assert url_for('main.service_settings', service_id=service_one['id']) in page
+    assert url_for('main.api_integration', service_id=service_one['id']) in page
 
 
 def test_menu_all_services_for_platform_admin_user(
+    client_request,
     mocker,
-    notify_admin,
     platform_admin_user,
     service_one,
     mock_get_service_templates,
@@ -1335,20 +1330,19 @@ def test_menu_all_services_for_platform_admin_user(
     mock_get_returned_letter_statistics_with_no_returned_letters,
     mock_get_free_sms_fragment_limit,
 ):
-    with notify_admin.test_request_context():
-        resp = _test_dashboard_menu(
-            mocker,
-            notify_admin,
-            platform_admin_user,
-            service_one,
-            [])
-        page = resp.get_data(as_text=True)
-        assert url_for('main.choose_template', service_id=service_one['id']) in page
-        assert url_for('main.manage_users', service_id=service_one['id']) in page
-        assert url_for('main.service_settings', service_id=service_one['id']) in page
-        assert url_for('main.view_notifications', service_id=service_one['id'], message_type='email') in page
-        assert url_for('main.view_notifications', service_id=service_one['id'], message_type='sms') in page
-        assert url_for('main.api_keys', service_id=service_one['id']) not in page
+    page = _test_dashboard_menu(
+        client_request,
+        mocker,
+        platform_admin_user,
+        service_one,
+        [])
+    page = str(page)
+    assert url_for('main.choose_template', service_id=service_one['id']) in page
+    assert url_for('main.manage_users', service_id=service_one['id']) in page
+    assert url_for('main.service_settings', service_id=service_one['id']) in page
+    assert url_for('main.view_notifications', service_id=service_one['id'], message_type='email') in page
+    assert url_for('main.view_notifications', service_id=service_one['id'], message_type='sms') in page
+    assert url_for('main.api_keys', service_id=service_one['id']) not in page
 
 
 def test_route_for_service_permissions(

tests/app/main/views/test_feedback.py

@@ -374,7 +374,7 @@ ids, params = zip(*[
     params, ids=ids
 )
 def test_redirects_to_triage(
-    client,
+    client_request,
     api_user_active,
     mocker,
     mock_get_user,
@@ -391,12 +391,15 @@ def test_redirects_to_triage(
         return_value=[{}, {}] if has_live_services else [],
     )
     mocker.patch('app.main.views.feedback.in_business_hours', return_value=is_in_business_hours)
-    if logged_in:
-        client.login(api_user_active)
+    if not logged_in:
+        client_request.logout()
 
-    response = client.get(url_for('main.feedback', ticket_type=ticket_type))
-    assert response.status_code == expected_status
-    assert response.location == expected_redirect(_external=True)
+    client_request.get(
+        'main.feedback',
+        ticket_type=ticket_type,
+        _expected_status=expected_status,
+        _expected_redirect=expected_redirect(_external=True),
+    )
 
 
 @pytest.mark.parametrize('ticket_type, expected_h1', (
@@ -573,7 +576,7 @@ def test_back_link_from_form(
     ]
 )
 def test_should_be_shown_the_bat_email(
-    client,
+    client_request,
     active_user_with_permissions,
     mocker,
     service_one,
@@ -590,16 +593,20 @@ def test_should_be_shown_the_bat_email(
 
     feedback_page = url_for('main.feedback', ticket_type=PROBLEM_TICKET_TYPE, severe=severe)
 
-    response = client.get(feedback_page)
-
-    assert response.status_code == expected_status_code
-    assert response.location == expected_redirect(_external=True)
+    client_request.logout()
+    client_request.get_url(
+        feedback_page,
+        _expected_status=expected_status_code,
+        _expected_redirect=expected_redirect(_external=True),
+    )
 
     # logged in users should never be redirected to the bat email page
-    client.login(active_user_with_permissions, mocker, service_one)
-    logged_in_response = client.get(feedback_page)
-    assert logged_in_response.status_code == expected_status_code_when_logged_in
-    assert logged_in_response.location == expected_redirect_when_logged_in(_external=True)
+    client_request.login(active_user_with_permissions)
+    client_request.get_url(
+        feedback_page,
+        _expected_status=expected_status_code_when_logged_in,
+        _expected_redirect=expected_redirect_when_logged_in(_external=True),
+    )
 
 
 @pytest.mark.parametrize(
@@ -625,7 +632,7 @@ def test_should_be_shown_the_bat_email(
     ]
 )
 def test_should_be_shown_the_bat_email_for_general_questions(
-    client,
+    client_request,
     active_user_with_permissions,
     mocker,
     service_one,
@@ -641,42 +648,45 @@ def test_should_be_shown_the_bat_email_for_general_questions(
 
     feedback_page = url_for('main.feedback', ticket_type=GENERAL_TICKET_TYPE, severe=severe)
 
-    response = client.get(feedback_page)
-
-    assert response.status_code == expected_status_code
-    assert response.location == expected_redirect(_external=True)
+    client_request.logout()
+    client_request.get_url(
+        feedback_page,
+        _expected_status=expected_status_code,
+        _expected_redirect=expected_redirect(_external=True),
+    )
 
     # logged in users should never be redirected to the bat email page
-    client.login(active_user_with_permissions, mocker, service_one)
-    logged_in_response = client.get(feedback_page)
-    assert logged_in_response.status_code == expected_status_code_when_logged_in
-    assert logged_in_response.location == expected_redirect_when_logged_in(_external=True)
+    client_request.login(active_user_with_permissions)
+    client_request.get_url(
+        feedback_page,
+        _expected_status=expected_status_code_when_logged_in,
+        _expected_redirect=expected_redirect_when_logged_in(_external=True),
+    )
 
 
 def test_bat_email_page(
-    client,
+    client_request,
     active_user_with_permissions,
     mocker,
     service_one,
 ):
-    bat_phone_page = url_for('main.bat_phone')
+    bat_phone_page = 'main.bat_phone'
 
-    response = client.get(bat_phone_page)
-    assert response.status_code == 200
+    client_request.logout()
+    page = client_request.get(bat_phone_page)
 
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
     assert page.select_one('.govuk-back-link').text == 'Back'
     assert page.select_one('.govuk-back-link')['href'] == url_for('main.support')
     assert page.select('main a')[1].text == 'Fill in this form'
     assert page.select('main a')[1]['href'] == url_for('main.feedback', ticket_type=PROBLEM_TICKET_TYPE, severe='no')
-    next_page_response = client.get(page.select('main a')[1]['href'])
-    next_page = BeautifulSoup(next_page_response.data.decode('utf-8'), 'html.parser')
+    next_page = client_request.get_url(page.select('main a')[1]['href'])
     assert next_page.h1.text.strip() == 'Report a problem'
 
-    client.login(active_user_with_permissions, mocker, service_one)
-    logged_in_response = client.get(bat_phone_page)
-    assert logged_in_response.status_code == 302
-    assert logged_in_response.location == url_for('main.feedback', ticket_type=PROBLEM_TICKET_TYPE, _external=True)
+    client_request.login(active_user_with_permissions)
+    client_request.get(
+        bat_phone_page,
+        _expected_redirect=url_for('main.feedback', ticket_type=PROBLEM_TICKET_TYPE, _external=True)
+    )
 
 
 @pytest.mark.parametrize('out_of_hours_emergency, email_address_provided, out_of_hours, message', (

tests/app/main/views/test_find_users.py

@@ -1,9 +1,7 @@
 import uuid
 
 import pytest
-from bs4 import BeautifulSoup
 from flask import url_for
-from lxml import html
 from notifications_python_client.errors import HTTPError
 
 from tests import user_json
@@ -92,11 +90,12 @@ def test_find_users_by_email_validates_against_empty_search_submission(
 
 
 def test_user_information_page_shows_information_about_user(
-    client,
+    client_request,
     platform_admin_user,
     mocker,
     fake_uuid,
 ):
+    client_request.login(platform_admin_user)
     user_service_one = uuid.uuid4()
     user_service_two = uuid.uuid4()
     mocker.patch('app.user_api_client.get_user', side_effect=[
@@ -112,32 +111,43 @@ def test_user_information_page_shows_information_about_user(
         ]},
         autospec=True
     )
-    client.login(platform_admin_user)
-    response = client.get(url_for('main.user_information', user_id=fake_uuid))
-    assert response.status_code == 200
+    page = client_request.get('main.user_information', user_id=fake_uuid)
 
-    document = html.fromstring(response.get_data(as_text=True))
+    assert normalize_spaces(page.select_one('h1').text) == 'Apple Bloom'
 
-    assert document.xpath("//h1/text()[normalize-space()='Apple Bloom']")
-    assert document.xpath("//p/text()[normalize-space()='test@gov.uk']")
-    assert document.xpath("//p/text()[normalize-space()='+447700900986']")
+    assert [
+        normalize_spaces(p.text) for p in page.select('main p')
+    ] == [
+        'test@gov.uk',
+        '+447700900986',
+        'Last logged in just now',
+    ]
 
-    assert document.xpath("//h2/text()[normalize-space()='Live services']")
-    assert document.xpath("//a/text()[normalize-space()='Nature Therapy']")
+    assert '0 failed login attempts' not in page.text
 
-    assert document.xpath("//h2/text()[normalize-space()='Trial mode services']")
-    assert document.xpath("//a/text()[normalize-space()='Fresh Orchard Juice']")
+    assert [
+        normalize_spaces(h2.text) for h2 in page.select('main h2')
+    ] == [
+        'Live services',
+        'Trial mode services',
+        'Last login',
+    ]
 
-    assert document.xpath("//h2/text()[normalize-space()='Last login']")
-    assert not document.xpath("//p/text()[normalize-space()='0 failed login attempts']")
+    assert [
+        normalize_spaces(a.text) for a in page.select('main li a')
+    ] == [
+        'Nature Therapy',
+        'Fresh Orchard Juice',
+    ]
 
 
 def test_user_information_page_displays_if_there_are_failed_login_attempts(
-    client,
+    client_request,
     platform_admin_user,
     mocker,
     fake_uuid,
 ):
+    client_request.login(platform_admin_user)
     mocker.patch('app.user_api_client.get_user', side_effect=[
         platform_admin_user,
         user_json(name="Apple Bloom", failed_login_count=2)
@@ -148,12 +158,11 @@ def test_user_information_page_displays_if_there_are_failed_login_attempts(
         return_value={'organisations': [], 'services': []},
         autospec=True
     )
-    client.login(platform_admin_user)
-    response = client.get(url_for('main.user_information', user_id=fake_uuid))
-    assert response.status_code == 200
+    page = client_request.get('main.user_information', user_id=fake_uuid)
 
-    document = html.fromstring(response.get_data(as_text=True))
-    assert document.xpath("//p/text()[normalize-space()='2 failed login attempts']")
+    assert normalize_spaces(page.select('main p')[-1].text) == (
+        '2 failed login attempts'
+    )
 
 
 def test_user_information_page_shows_archive_link_for_active_users(
@@ -174,19 +183,20 @@ def test_user_information_page_shows_archive_link_for_active_users(
 
 def test_user_information_page_does_not_show_archive_link_for_inactive_users(
     mocker,
-    client,
+    client_request,
     platform_admin_user,
     mock_get_organisations_and_services_for_user,
 ):
-    inactive_user = user_json(state='inactive')
+    inactive_user_id = uuid.uuid4()
+    inactive_user = user_json(id_=inactive_user_id, state='inactive')
+    client_request.login(platform_admin_user)
     mocker.patch('app.user_api_client.get_user', side_effect=[platform_admin_user, inactive_user], autospec=True)
-    client.login(platform_admin_user)
-    response = client.get(
-        url_for('main.user_information', user_id=inactive_user['id'])
-    )
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
 
-    archive_url = url_for('main.archive_user', user_id=inactive_user['id'])
+    page = client_request.get(
+        'main.user_information', user_id=inactive_user_id
+    )
+
+    archive_url = url_for('main.archive_user', user_id=inactive_user_id)
     assert not page.find('a', {'href': archive_url})
 
 

tests/app/main/views/test_letters.py

@@ -34,7 +34,7 @@ def test_letters_access_restricted(
 
 @pytest.mark.parametrize('url', letters_urls)
 def test_letters_lets_in_without_permission(
-    client,
+    client_request,
     mocker,
     mock_login,
     mock_has_permissions,
@@ -46,11 +46,10 @@ def test_letters_lets_in_without_permission(
     service_one['permissions'] = ['letter']
     mocker.patch('app.service_api_client.get_service', return_value={"data": service_one})
 
-    client.login(api_user_active)
-    response = client.get(url(service_id=service_one['id']))
+    client_request.login(api_user_active)
+    client_request.get_url(url(service_id=service_one['id']))
 
     assert api_user_active['permissions'] == {}
-    assert response.status_code == 200
 
 
 @pytest.mark.parametrize('permissions, choices', [

tests/app/main/views/test_providers.py

@@ -1,10 +1,8 @@
 import copy
-import re
 from datetime import datetime
 from unittest.mock import call
 
 import pytest
-from bs4 import BeautifulSoup
 from flask import url_for
 from freezegun import freeze_time
 
@@ -146,15 +144,14 @@ stub_provider_history = {
 
 
 def test_should_show_all_providers(
-    client,
+    client_request,
     platform_admin_user,
     mocker,
 ):
     mocker.patch('app.provider_client.get_all_providers', return_value=copy.deepcopy(stub_providers))
 
-    client.login(platform_admin_user, mocker)
-    response = client.get(url_for('main.view_providers'))
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    client_request.login(platform_admin_user)
+    page = client_request.get('main.view_providers')
 
     h1 = [header.text.strip() for header in page.find_all('h1')]
 
@@ -259,17 +256,15 @@ def test_add_monthly_traffic():
 
 
 def test_should_show_edit_provider_form(
-    client,
+    client_request,
     platform_admin_user,
     mocker,
     fake_uuid,
 ):
     mocker.patch('app.provider_client.get_provider_by_id', return_value=copy.deepcopy(stub_provider))
 
-    client.login(platform_admin_user, mocker)
-    response = client.get(url_for('main.edit_provider', provider_id=fake_uuid))
-
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    client_request.login(platform_admin_user)
+    page = client_request.get('main.edit_provider', provider_id=fake_uuid)
 
     h1 = [header.text.strip() for header in page.find_all('h1')]
 
@@ -283,102 +278,115 @@ def test_should_show_edit_provider_form(
 
 
 def test_should_show_error_on_bad_provider_priority(
-    client,
+    client_request,
     platform_admin_user,
     mocker,
 ):
     mocker.patch('app.provider_client.get_provider_by_id', return_value=copy.deepcopy(stub_provider))
 
-    client.login(platform_admin_user, mocker)
-    response = client.post(
-        url_for('main.edit_provider', provider_id=stub_provider['provider_details']['id']),
-        data={'priority': "not valid"})
+    client_request.login(platform_admin_user)
+    page = client_request.post(
+        'main.edit_provider',
+        provider_id=stub_provider['provider_details']['id'],
+        _data={'priority': "not valid"},
+        _expected_status=200,
+    )
 
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-    assert response.status_code == 200
-    assert "Not a valid integer value" in str(page.find_all("span", {"class": re.compile(r"error-message")})[0])
+    assert normalize_spaces(
+        page.select_one('.govuk-error-message').text
+    ) == "Error: Not a valid integer value."
 
 
 def test_should_show_error_on_negative_provider_priority(
-    client,
+    client_request,
     platform_admin_user,
     mocker,
 ):
     mocker.patch('app.provider_client.get_provider_by_id', return_value=copy.deepcopy(stub_provider))
 
-    client.login(platform_admin_user, mocker)
-    response = client.post(
-        url_for('main.edit_provider', provider_id=stub_provider['provider_details']['id']),
-        data={'priority': -1})
+    client_request.login(platform_admin_user)
+    page = client_request.post(
+        'main.edit_provider',
+        provider_id=stub_provider['provider_details']['id'],
+        _data={'priority': -1},
+        _expected_status=200,
+    )
 
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-    assert response.status_code == 200
-    assert "Must be between 1 and 100" in str(page.find_all("span", {"class": re.compile(r"error-message")})[0])
+    assert normalize_spaces(
+        page.select_one('.govuk-error-message').text
+    ) == "Error: Must be between 1 and 100"
 
 
 def test_should_show_error_on_too_big_provider_priority(
-    client,
+    client_request,
     platform_admin_user,
     mocker,
 ):
     mocker.patch('app.provider_client.get_provider_by_id', return_value=copy.deepcopy(stub_provider))
 
-    client.login(platform_admin_user, mocker)
-    response = client.post(
-        url_for('main.edit_provider', provider_id=stub_provider['provider_details']['id']),
-        data={'priority': 101})
+    client_request.login(platform_admin_user)
+    page = client_request.post(
+        'main.edit_provider',
+        provider_id=stub_provider['provider_details']['id'],
+        _data={'priority': 101},
+        _expected_status=200,
+    )
 
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-    assert response.status_code == 200
-    assert "Must be between 1 and 100" in str(page.find_all("span", {"class": re.compile(r"error-message")})[0])
+    assert normalize_spaces(
+        page.select_one('.govuk-error-message').text
+    ) == "Error: Must be between 1 and 100"
 
 
 def test_should_show_error_on_too_little_provider_priority(
-    client,
+    client_request,
     platform_admin_user,
     mocker,
 ):
     mocker.patch('app.provider_client.get_provider_by_id', return_value=copy.deepcopy(stub_provider))
 
-    client.login(platform_admin_user, mocker)
-    response = client.post(
-        url_for('main.edit_provider', provider_id=stub_provider['provider_details']['id']),
-        data={'priority': 0})
+    client_request.login(platform_admin_user)
+    page = client_request.post(
+        'main.edit_provider',
+        provider_id=stub_provider['provider_details']['id'],
+        _data={'priority': 0},
+        _expected_status=200,
+    )
 
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
-    assert response.status_code == 200
-    assert "Must be between 1 and 100" in str(page.find_all("span", {"class": re.compile(r"error-message")})[0])
+    assert normalize_spaces(
+        page.select_one('.govuk-error-message').text
+    ) == "Error: Must be between 1 and 100"
 
 
 def test_should_update_provider_priority(
-    client,
+    client_request,
     platform_admin_user,
     mocker,
 ):
     mocker.patch('app.provider_client.get_provider_by_id', return_value=copy.deepcopy(stub_provider))
     mocker.patch('app.provider_client.update_provider', return_value=copy.deepcopy(stub_provider))
 
-    client.login(platform_admin_user, mocker)
-    response = client.post(
-        url_for('main.edit_provider', provider_id=stub_provider['provider_details']['id']),
-        data={'priority': 2})
+    client_request.login(platform_admin_user)
+    client_request.post(
+        'main.edit_provider',
+        provider_id=stub_provider['provider_details']['id'],
+        _data={'priority': 2},
+        _expected_redirect='http://localhost/providers',
+    )
 
     app.provider_client.update_provider.assert_called_with(stub_provider['provider_details']['id'], 2)
-    assert response.status_code == 302
-    assert response.location == 'http://localhost/providers'
 
 
 def test_should_show_provider_version_history(
-    client,
+    client_request,
     platform_admin_user,
     mocker
 ):
     mocker.patch('app.provider_client.get_provider_versions', return_value=copy.deepcopy(stub_provider_history))
 
-    client.login(platform_admin_user, mocker)
-    response = client.get(
-        url_for('main.view_provider', provider_id=stub_provider_history['data'][0]['id']))
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    client_request.login(platform_admin_user)
+    page = client_request.get(
+        'main.view_provider', provider_id=stub_provider_history['data'][0]['id']
+    )
 
     table = page.find('table')
     table_rows = table.find_all('tr')
@@ -386,8 +394,6 @@ def test_should_show_provider_version_history(
     first_row = table_rows[1].find_all('td')
     second_row = table_rows[2].find_all('td')
 
-    assert response.status_code == 200
-
     assert page.find_all('h1')[0].text.strip() == stub_provider_history['data'][0]["display_name"]
     assert len(table_rows) == 3
 

tests/app/main/views/test_service_settings.py

@@ -6,7 +6,6 @@ from urllib.parse import parse_qs, urlparse
 from uuid import UUID, uuid4
 
 import pytest
-from bs4 import BeautifulSoup
 from flask import url_for
 from freezegun import freeze_time
 from notifications_python_client.errors import HTTPError
@@ -118,7 +117,7 @@ def mock_get_service_settings_page_common(
     ]),
 ])
 def test_should_show_overview(
-        client,
+        client_request,
         mocker,
         api_user_active,
         no_reply_to_email_addresses,
@@ -137,12 +136,11 @@ def test_should_show_overview(
     )
     mocker.patch('app.service_api_client.get_service', return_value={'data': service_one})
 
-    client.login(user, mocker, service_one)
-    response = client.get(url_for(
+    client_request.login(user, service_one)
+    page = client_request.get(
         'main.service_settings', service_id=SERVICE_ONE_ID
-    ))
-    assert response.status_code == 200
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    )
+
     assert page.find('h1').text == 'Settings'
     rows = page.select('tr')
     assert len(rows) == len(expected_rows)
@@ -152,7 +150,7 @@ def test_should_show_overview(
 
 
 def test_platform_admin_sees_only_relevant_settings_for_broadcast_service(
-        client,
+        client_request,
         mocker,
         api_user_active,
         no_reply_to_email_addresses,
@@ -170,12 +168,11 @@ def test_platform_admin_sees_only_relevant_settings_for_broadcast_service(
     )
     mocker.patch('app.service_api_client.get_service', return_value={'data': service_one})
 
-    client.login(create_platform_admin_user(), mocker, service_one)
-    response = client.get(url_for(
+    client_request.login(create_platform_admin_user(), service_one)
+    page = client_request.get(
         'main.service_settings', service_id=SERVICE_ONE_ID
-    ))
-    assert response.status_code == 200
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    )
+
     assert page.find('h1').text == 'Settings'
     rows = page.select('tr')
 
@@ -212,7 +209,7 @@ def test_platform_admin_sees_only_relevant_settings_for_broadcast_service(
     ]
 )
 def test_platform_admin_sees_correct_description_of_broadcast_service_setting(
-        client,
+        client_request,
         mocker,
         api_user_active,
         no_reply_to_email_addresses,
@@ -236,12 +233,11 @@ def test_platform_admin_sees_correct_description_of_broadcast_service_setting(
     )
     mocker.patch('app.service_api_client.get_service', return_value={'data': service_one})
 
-    client.login(create_platform_admin_user(), mocker, service_one)
-    response = client.get(url_for(
+    client_request.login(create_platform_admin_user(), service_one)
+    page = client_request.get(
         'main.service_settings', service_id=SERVICE_ONE_ID
-    ))
-    assert response.status_code == 200
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    )
+
     broadcast_setting_row = page.find(string=re.compile("Emergency alerts")).find_parent('tr')
     broadcast_setting_description = broadcast_setting_row.select('td')[1].text
     assert normalize_spaces(broadcast_setting_description) == expected_text
@@ -398,7 +394,7 @@ def test_send_files_by_email_row_on_settings_page(
     ]),
 ])
 def test_should_show_overview_for_service_with_more_things_set(
-        client,
+        client_request,
         active_user_with_permissions,
         mocker,
         service_one,
@@ -410,13 +406,12 @@ def test_should_show_overview_for_service_with_more_things_set(
         permissions,
         expected_rows
 ):
-    client.login(active_user_with_permissions, mocker, service_one)
+    client_request.login(active_user_with_permissions)
     service_one['permissions'] = permissions
     service_one['email_branding'] = uuid4()
-    response = client.get(url_for(
+    page = client_request.get(
         'main.service_settings', service_id=service_one['id']
-    ))
-    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    )
     for index, row in enumerate(expected_rows):
         assert row == " ".join(page.find_all('tr')[index + 1].text.split())
 

tests/app/main/views/test_templates.py

@@ -1793,7 +1793,7 @@ def test_should_not_allow_template_edits_without_correct_permission(
 
 
 def test_should_403_when_edit_template_with_process_type_of_priority_for_non_platform_admin(
-    client,
+    client_request,
     active_user_with_permissions,
     mocker,
     mock_get_service_template,
@@ -1802,7 +1802,7 @@ def test_should_403_when_edit_template_with_process_type_of_priority_for_non_pla
     service_one,
 ):
     service_one['users'] = [active_user_with_permissions]
-    client.login(active_user_with_permissions, mocker, service_one)
+    client_request.login(active_user_with_permissions)
     mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions])
     template_id = fake_uuid
     data = {
@@ -1813,16 +1813,18 @@ def test_should_403_when_edit_template_with_process_type_of_priority_for_non_pla
         'service': service_one['id'],
         'process_type': 'priority'
     }
-    response = client.post(url_for(
+    client_request.post(
         '.edit_service_template',
         service_id=service_one['id'],
-        template_id=template_id), data=data)
-    assert response.status_code == 403
+        template_id=template_id,
+        _data=data,
+        _expected_status=403,
+    )
     assert mock_update_service_template.called is False
 
 
 def test_should_403_when_create_template_with_process_type_of_priority_for_non_platform_admin(
-    client,
+    client_request,
     active_user_with_permissions,
     mocker,
     mock_get_service_template,
@@ -1831,7 +1833,7 @@ def test_should_403_when_create_template_with_process_type_of_priority_for_non_p
     service_one,
 ):
     service_one['users'] = [active_user_with_permissions]
-    client.login(active_user_with_permissions, mocker, service_one)
+    client_request.login(active_user_with_permissions, service_one)
     mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions])
     template_id = fake_uuid
     data = {
@@ -1842,11 +1844,13 @@ def test_should_403_when_create_template_with_process_type_of_priority_for_non_p
         'service': service_one['id'],
         'process_type': 'priority'
     }
-    response = client.post(url_for(
+    client_request.post(
         '.add_service_template',
         service_id=service_one['id'],
-        template_type='sms'), data=data)
-    assert response.status_code == 403
+        template_type='sms',
+        _data=data,
+        _expected_status=403,
+    )
     assert mock_update_service_template.called is False
 
 

tests/app/utils/test_user.py

@@ -7,7 +7,7 @@ from app.utils.user import user_has_permissions
 
 
 def _test_permissions(
-    client,
+    client_request,
     usr,
     permissions,
     will_succeed,
@@ -15,7 +15,7 @@ def _test_permissions(
 ):
     request.view_args.update({'service_id': 'foo'})
     if usr:
-        client.login(usr)
+        client_request.login(usr)
 
     decorator = user_has_permissions(*permissions, **(kwargs or {}))
     decorated_index = decorator(index)
@@ -34,93 +34,93 @@ def _test_permissions(
 
 
 def test_user_has_permissions_on_endpoint_fail(
-    client,
+    client_request,
     mocker,
     mock_get_service,
 ):
     user = _user_with_permissions()
     mocker.patch('app.user_api_client.get_user', return_value=user)
     _test_permissions(
-        client,
+        client_request,
         user,
         ['send_messages'],
         will_succeed=False)
 
 
 def test_user_has_permissions_success(
-    client,
+    client_request,
     mocker,
 ):
     user = _user_with_permissions()
     mocker.patch('app.user_api_client.get_user', return_value=user)
     _test_permissions(
-        client,
+        client_request,
         user,
         ['manage_service'],
         will_succeed=True)
 
 
 def test_user_has_permissions_or(
-    client,
+    client_request,
     mocker,
 ):
     user = _user_with_permissions()
     mocker.patch('app.user_api_client.get_user', return_value=user)
     _test_permissions(
-        client,
+        client_request,
         user,
         ['send_messages', 'manage_service'],
         will_succeed=True)
 
 
 def test_user_has_permissions_multiple(
-    client,
+    client_request,
     mocker,
 ):
     user = _user_with_permissions()
     mocker.patch('app.user_api_client.get_user', return_value=user)
     _test_permissions(
-        client,
+        client_request,
         user,
         ['manage_templates', 'manage_service'],
         will_succeed=True)
 
 
 def test_exact_permissions(
-    client,
+    client_request,
     mocker,
 ):
     user = _user_with_permissions()
     mocker.patch('app.user_api_client.get_user', return_value=user)
     _test_permissions(
-        client,
+        client_request,
         user,
         ['manage_service', 'manage_templates'],
         will_succeed=True)
 
 
 def test_platform_admin_user_can_access_page_that_has_no_permissions(
-    client,
+    client_request,
     platform_admin_user,
     mocker,
 ):
     mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
     _test_permissions(
-        client,
+        client_request,
         platform_admin_user,
         [],
         will_succeed=True)
 
 
 def test_platform_admin_user_can_not_access_page(
-    client,
+    client_request,
     platform_admin_user,
     mocker,
     mock_get_service,
 ):
     mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
     _test_permissions(
-        client,
+        client_request,
         platform_admin_user,
         [],
         will_succeed=False,
@@ -128,25 +128,24 @@ def test_platform_admin_user_can_not_access_page(
 
 
 def test_no_user_returns_401_unauth(
-    client
+    client_request
 ):
-    from flask_login import current_user
-    assert not current_user.is_authenticated
+    client_request.logout()
     _test_permissions(
-        client,
+        client_request,
         None,
         [],
         will_succeed=False)
 
 
 def test_user_has_permissions_for_organisation(
-    client,
+    client_request,
     mocker,
 ):
     user = _user_with_permissions()
     user['organisations'] = ['org_1', 'org_2']
     mocker.patch('app.user_api_client.get_user', return_value=user)
-    client.login(user)
+    client_request.login(user)
 
     request.view_args = {'org_id': 'org_2'}
 
@@ -158,13 +157,13 @@ def test_user_has_permissions_for_organisation(
 
 
 def test_platform_admin_can_see_orgs_they_dont_have(
-    client,
+    client_request,
     platform_admin_user,
     mocker,
 ):
     platform_admin_user['organisations'] = []
     mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
-    client.login(platform_admin_user)
+    client_request.login(platform_admin_user)
 
     request.view_args = {'org_id': 'org_2'}
 
@@ -176,12 +175,12 @@ def test_platform_admin_can_see_orgs_they_dont_have(
 
 
 def test_cant_use_decorator_without_view_args(
-    client,
+    client_request,
     platform_admin_user,
     mocker,
 ):
     mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
-    client.login(platform_admin_user)
+    client_request.login(platform_admin_user)
 
     request.view_args = {}
 
@@ -194,13 +193,13 @@ def test_cant_use_decorator_without_view_args(
 
 
 def test_user_doesnt_have_permissions_for_organisation(
-    client,
+    client_request,
     mocker,
 ):
     user = _user_with_permissions()
     user['organisations'] = ['org_1', 'org_2']
     mocker.patch('app.user_api_client.get_user', return_value=user)
-    client.login(user)
+    client_request.login(user)
 
     request.view_args = {'org_id': 'org_3'}
 
@@ -213,12 +212,12 @@ def test_user_doesnt_have_permissions_for_organisation(
 
 
 def test_user_with_no_permissions_to_service_goes_to_templates(
-        client,
+        client_request,
         mocker
 ):
     user = _user_with_permissions()
     mocker.patch('app.user_api_client.get_user', return_value=user)
-    client.login(user)
+    client_request.login(user)
     request.view_args = {'service_id': 'bar'}
 
     @user_has_permissions()