Skip to content

Commit

Permalink
Fix oppia#4829: Reduce code redundancy in backend testing (oppia#5838)
Browse files Browse the repository at this point in the history 
* Replaced testapp.get with appropriate functions

* Add test cases

* Function get_response

* Function get_response_without_checking_for_errors

* Fixed Errors

* Fixed lint checks

* removed further redundancy

* Lint checks

* Lint checks

* made params only dict type

* Minor changes
  • Loading branch information
@kevinlee12
Rishav Chakraborty authored and kevinlee12 committed Jan 6, 2019
1 parent b35418b commit 35c2f13
Show file tree
Hide file tree
Showing 36 changed files with 683 additions and 783 deletions.
39 changes: 17 additions & 22 deletions core/controllers/admin_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,23 @@ def setUp(self):
def test_admin_page_rights(self):
"""Test access rights to the admin page."""

response = self.testapp.get('/admin')
self.assertEqual(response.status_int, 302)
self.get_html_response('/admin', expected_status_int=302)

# Login as a non-admin.
self.login(self.EDITOR_EMAIL)
response = self.testapp.get('/admin', expect_errors=True)
self.assertEqual(response.status_int, 401)
self.get_html_response('/admin', expected_status_int=401)
self.logout()

# Login as an admin.
self.login(self.ADMIN_EMAIL, is_super_admin=True)
response = self.testapp.get('/admin')
self.assertEqual(response.status_int, 200)
self.get_html_response('/admin')
self.logout()

def test_change_configuration_property(self):
"""Test that configuration properties can be changed."""

self.login(self.ADMIN_EMAIL, is_super_admin=True)
response = self.testapp.get('/admin')
response = self.get_html_response('/admin')
csrf_token = self.get_csrf_token_from_response(response)

response_dict = self.get_json('/adminhandler')
Expand Down Expand Up @@ -87,11 +84,11 @@ def test_change_about_page_config_property(self):
"""Test that config property values are changed correctly."""
new_config_value = 'new_config_value'

response = self.testapp.get('/about')
response = self.get_html_response('/about')
self.assertNotIn(new_config_value, response.body)

self.login(self.ADMIN_EMAIL, is_super_admin=True)
response = self.testapp.get('/admin')
response = self.get_html_response('/admin')
csrf_token = self.get_csrf_token_from_response(response)
self.post_json(
'/adminhandler', {
Expand All @@ -101,7 +98,7 @@ def test_change_about_page_config_property(self):
}
}, csrf_token=csrf_token)

response = self.testapp.get('/about')
response = self.get_html_response('/about')
self.assertIn(new_config_value, response.body)


Expand All @@ -111,7 +108,7 @@ class GenerateDummyExplorationsTest(test_utils.GenericTestBase):
def test_generate_count_greater_than_publish_count(self):
self.signup(self.ADMIN_EMAIL, self.ADMIN_USERNAME)
self.login(self.ADMIN_EMAIL, is_super_admin=True)
response = self.testapp.get('/admin')
response = self.get_html_response('/admin')
csrf_token = self.get_csrf_token_from_response(response)
self.post_json(
'/adminhandler', {
Expand All @@ -127,7 +124,7 @@ def test_generate_count_greater_than_publish_count(self):
def test_generate_count_equal_to_publish_count(self):
self.signup(self.ADMIN_EMAIL, self.ADMIN_USERNAME)
self.login(self.ADMIN_EMAIL, is_super_admin=True)
response = self.testapp.get('/admin')
response = self.get_html_response('/admin')
csrf_token = self.get_csrf_token_from_response(response)
self.post_json(
'/adminhandler', {
Expand All @@ -143,17 +140,15 @@ def test_generate_count_equal_to_publish_count(self):
def test_generate_count_less_than_publish_count(self):
self.signup(self.ADMIN_EMAIL, self.ADMIN_USERNAME)
self.login(self.ADMIN_EMAIL, is_super_admin=True)
response = self.testapp.get('/admin')
response = self.get_html_response('/admin')
csrf_token = self.get_csrf_token_from_response(response)
generated_exps_response = self.post_json(
'/adminhandler', {
'action': 'generate_dummy_explorations',
'num_dummy_exps_to_generate': 2,
'num_dummy_exps_to_publish': 5
},
csrf_token=csrf_token,
expect_errors=True,
expected_status_int=400)
csrf_token=csrf_token, expected_status_int=400)
self.assertEqual(generated_exps_response['status_code'], 400)
generated_exps = exp_services.get_all_exploration_summaries()
published_exps = exp_services.get_recently_published_exp_summaries(5)
Expand Down Expand Up @@ -185,12 +180,12 @@ def test_view_and_update_role(self):
response_dict, {'user1': feconf.ROLE_ID_EXPLORATION_EDITOR})

# Check role correctly gets updated.
response = self.testapp.get(feconf.ADMIN_URL)
response = self.get_html_response(feconf.ADMIN_URL)
csrf_token = self.get_csrf_token_from_response(response)
response_dict = self.post_json(
feconf.ADMIN_ROLE_HANDLER_URL,
{'role': feconf.ROLE_ID_MODERATOR, 'username': username},
csrf_token=csrf_token, expect_errors=False,
csrf_token=csrf_token,
expected_status_int=200)
self.assertEqual(response_dict, {})

Expand All @@ -210,15 +205,15 @@ def test_invalid_username_in_view_and_update_role(self):
response = self.get_json(
feconf.ADMIN_ROLE_HANDLER_URL,
params={'method': 'username', 'username': username},
expected_status_int=400, expect_errors=True)
expected_status_int=400)

# Trying to update role of non-existent user.
response = self.testapp.get(feconf.ADMIN_URL)
response = self.get_html_response(feconf.ADMIN_URL)
csrf_token = self.get_csrf_token_from_response(response)
response = self.post_json(
feconf.ADMIN_ROLE_HANDLER_URL,
{'role': feconf.ROLE_ID_MODERATOR, 'username': username},
csrf_token=csrf_token, expect_errors=True,
csrf_token=csrf_token,
expected_status_int=400)


Expand Down Expand Up @@ -295,7 +290,7 @@ def test_that_handler_raises_exception(self):

response = self.get_json(
'/explorationdataextractionhandler', params=payload,
expected_status_int=400, expect_errors=True)
expected_status_int=400)

self.assertEqual(
response['error'],
Expand Down
55 changes: 23 additions & 32 deletions core/controllers/base_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ def test_dev_indicator_appears_in_dev_and_not_in_production(self):
"""Test dev indicator appears in dev and not in production."""

with self.swap(constants, 'DEV_MODE', True):
response = self.testapp.get(feconf.LIBRARY_INDEX_URL)
response = self.get_html_response(feconf.LIBRARY_INDEX_URL)
self.assertIn('<div ng-if="DEV_MODE" class="oppia-dev-mode">',
response.body)

with self.swap(constants, 'DEV_MODE', False):
response = self.testapp.get(feconf.LIBRARY_INDEX_URL)
response = self.get_html_response(feconf.LIBRARY_INDEX_URL)
self.assertIn('<div ng-if="DEV_MODE" class="oppia-dev-mode">',
response.body)

Expand All @@ -95,9 +95,8 @@ def test_that_no_get_results_in_500_error(self):
url = re.sub('<([^/^:]+)>', 'abc123', url)

# Some of these will 404 or 302. This is expected.
response = self.testapp.get(url, expect_errors=True)
self.assertIn(
response.status_int, [200, 302, 400, 401, 404], msg=url)
self.get_response_without_checking_for_errors(
url, [200, 302, 400, 401, 404])

# TODO(sll): Add similar tests for POST, PUT, DELETE.
# TODO(sll): Set a self.payload attr in the BaseHandler for
Expand All @@ -107,25 +106,22 @@ def test_that_no_get_results_in_500_error(self):
def test_requests_for_invalid_paths(self):
"""Test that requests for invalid paths result in a 404 error."""

response = self.testapp.get('/library/extra', expect_errors=True)
self.assertEqual(response.status_int, 404)
self.get_html_response(
'/library/extra', expected_status_int=404)

response = self.testapp.get('/library/data/extra', expect_errors=True)
self.assertEqual(response.status_int, 404)
self.get_html_response(
'/library/data/extra', expected_status_int=404)

response = self.testapp.post(
'/library/extra', params={}, expect_errors=True)
self.assertEqual(response.status_int, 404)
self.post_json(
'/library/extra', payload={}, expected_status_int=404)

response = self.testapp.put(
'/library/extra', params={}, expect_errors=True)
self.assertEqual(response.status_int, 404)
self.put_json(
'/library/extra', payload={}, expected_status_int=404)

def test_redirect_in_logged_out_states(self):
"""Test for a redirect in logged out state on '/'."""

response = self.testapp.get('/')
self.assertEqual(response.status_int, 302)
response = self.get_html_response('/', expected_status_int=302)
self.assertIn('splash', response.headers['location'])

def test_root_redirect_rules_for_logged_in_learners(self):
Expand All @@ -134,8 +130,7 @@ def test_root_redirect_rules_for_logged_in_learners(self):
# Since by default the homepage for all logged in users is the
# learner dashboard, going to '/' should redirect to the learner
# dashboard page.
response = self.testapp.get('/')
self.assertEqual(response.status_int, 302)
response = self.get_html_response('/', expected_status_int=302)
self.assertIn('learner_dashboard', response.headers['location'])
self.logout()

Expand All @@ -152,8 +147,7 @@ def test_root_redirect_rules_for_users_with_no_user_contribution_model(
# Since by default the homepage for all logged in users is the
# learner dashboard, going to '/' should redirect to the learner
# dashboard page.
response = self.testapp.get('/')
self.assertEqual(response.status_int, 302)
response = self.get_html_response('/', expected_status_int=302)
self.assertIn('learner_dashboard', response.headers['location'])
self.logout()

Expand All @@ -166,7 +160,7 @@ def test_root_redirect_rules_for_logged_in_creators(self):

# Since the default dashboard has been set as creator dashboard, going
# to '/' should redirect to the creator dashboard.
response = self.testapp.get('/')
response = self.get_html_response('/', expected_status_int=302)
self.assertIn('creator_dashboard', response.headers['location'])

def test_root_redirect_rules_for_logged_in_editors(self):
Expand Down Expand Up @@ -196,8 +190,7 @@ def test_root_redirect_rules_for_logged_in_editors(self):

# Since user has edited one exploration created by another user,
# going to '/' should redirect to the dashboard page.
response = self.testapp.get('/')
self.assertEqual(response.status_int, 302)
response = self.get_html_response('/', expected_status_int=302)
self.assertIn('dashboard', response.headers['location'])
self.logout()

Expand Down Expand Up @@ -280,8 +273,7 @@ def setUp(self):
def test_jinja_autoescaping(self):
form_url = '<[angular_tag]> x{{51 * 3}}y'
with self.swap(feconf, 'SITE_FEEDBACK_FORM_URL', form_url):
response = self.testapp.get('/fake')
self.assertEqual(response.status_int, 200)
response = self.get_html_response('/fake')

self.assertIn('&lt;[angular_tag]&gt;', response.body)
self.assertNotIn('<[angular_tag]>', response.body)
Expand Down Expand Up @@ -322,7 +314,6 @@ def setUp(self):

def test_downloadable(self):
response = self.testapp.get('/mock')
self.assertEqual(response.status_int, 200)
self.assertEqual(
response.content_disposition,
'attachment; filename=example.pdf')
Expand All @@ -338,10 +329,10 @@ def test_logout_page(self):
# Logout with valid query arg. This test only validates that the login
# cookies have expired after hitting the logout url.
current_page = '/explore/0'
response = self.testapp.get(current_page)
self.assertEqual(response.status_int, 200)
response = self.testapp.get(current_user_services.create_logout_url(
current_page))
response = self.get_html_response(current_page)
response = self.get_html_response(
current_user_services.create_logout_url(
current_page), expected_status_int=302)
expiry_date = response.headers['Set-Cookie'].rsplit('=', 1)

self.assertTrue(
Expand Down Expand Up @@ -552,7 +543,7 @@ def test_error_response_for_get_request_of_type_json_has_json_format(self):
self.testapp = webtest.TestApp(app)

response = self.get_json(
'/fake', expect_errors=True, expected_status_int=500)
'/fake', expected_status_int=500)
self.assertTrue(isinstance(response, dict))


Expand Down
26 changes: 13 additions & 13 deletions core/controllers/classifier_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def test_trained_classifier_handler(self):
# Normal end-to-end test.
self.post_json(
'/ml/trainedclassifierhandler', self.payload,
expect_errors=False, expected_status_int=200)
expected_status_int=200)
classifier_training_jobs = (
classifier_services.get_classifier_training_jobs(
self.exp_id, self.exploration.version, ['Home']))
Expand Down Expand Up @@ -171,7 +171,7 @@ def mock_get_classifier_training_job_by_id(_):
# Post ML Job.
self.post_json(
'/ml/trainedclassifierhandler', self.payload,
expect_errors=True, expected_status_int=500)
expected_status_int=500)

# Check that there are now emails sent.
messages = self.mail_stub.get_sent_messages(
Expand All @@ -189,28 +189,28 @@ def test_error_on_prod_mode_and_default_vm_id(self):
with self.swap(constants, 'DEV_MODE', False):
self.post_json(
'/ml/trainedclassifierhandler', self.payload,
expect_errors=True, expected_status_int=401)
expected_status_int=401)

def test_error_on_different_signatures(self):
# Altering data to result in different signatures.
self.payload['message']['job_id'] = 'different_job_id'
self.post_json(
'/ml/trainedclassifierhandler', self.payload,
expect_errors=True, expected_status_int=401)
expected_status_int=401)

def test_error_on_invalid_job_id_in_message(self):
# Altering message dict to result in invalid dict.
self.payload['message']['job_id'] = 1
self.post_json(
'/ml/trainedclassifierhandler', self.payload,
expect_errors=True, expected_status_int=400)
expected_status_int=400)

def test_error_on_invalid_classifier_data_in_message(self):
# Altering message dict to result in invalid dict.
self.payload['message']['classifier_data_with_floats_stringified'] = 1
self.post_json(
'/ml/trainedclassifierhandler', self.payload,
expect_errors=True, expected_status_int=400)
expected_status_int=400)

def test_error_on_failed_training_job_status(self):
classifier_training_job_model = (
Expand All @@ -222,7 +222,7 @@ def test_error_on_failed_training_job_status(self):

self.post_json(
'/ml/trainedclassifierhandler', self.payload,
expect_errors=True, expected_status_int=500)
expected_status_int=500)

def test_error_on_exception_in_store_classifier_data(self):
classifier_training_job_model = (
Expand All @@ -233,7 +233,7 @@ def test_error_on_exception_in_store_classifier_data(self):

self.post_json(
'/ml/trainedclassifierhandler', self.payload,
expect_errors=True, expected_status_int=500)
expected_status_int=500)


class NextJobHandlerTest(test_utils.GenericTestBase):
Expand Down Expand Up @@ -280,13 +280,13 @@ def setUp(self):
def test_next_job_handler(self):
json_response = self.post_json(
'/ml/nextjobhandler',
self.payload, expect_errors=False,
self.payload,
expected_status_int=200)
self.assertEqual(json_response, self.expected_response)
classifier_services.mark_training_jobs_failed([self.job_id])
json_response = self.post_json(
'/ml/nextjobhandler',
self.payload, expect_errors=False,
self.payload,
expected_status_int=200)
self.assertEqual(json_response, {})

Expand All @@ -295,18 +295,18 @@ def test_error_on_prod_mode_and_default_vm_id(self):
with self.swap(constants, 'DEV_MODE', False):
self.post_json(
'/ml/nextjobhandler', self.payload,
expect_errors=True, expected_status_int=401)
expected_status_int=401)

def test_error_on_modified_message(self):
# Altering data to result in different signatures.
self.payload['message'] = 'different'
self.post_json(
'/ml/nextjobhandler', self.payload,
expect_errors=True, expected_status_int=401)
expected_status_int=401)

def test_error_on_invalid_vm_id(self):
# Altering vm_id to result in invalid signature.
self.payload['vm_id'] = 1
self.post_json(
'/ml/nextjobhandler', self.payload,
expect_errors=True, expected_status_int=401)
expected_status_int=401)
Loading

0 comments on commit 35c2f13

Please sign in to comment.