Skip to content

Commit

Permalink
Fix #5004: Modify keyword arguments checker (#5305)
Browse files Browse the repository at this point in the history
* Modify checker, tests and fix scripts

* Fix scripts

* Fix root-level scripts

* Fix extensions scripts

* Fix core/controllers scripts

* Fix core/domain scripts

* Fix core/storage scripts

* Fix core/platform scripts

* Fix core/tests scripts

* Fix core/controllers/question_editor_test

* Lint fix in utils

* TODO: Modify tests

* Run pre-commit script for all files

* Add period after comment

* Minor fix

* Modify tests and other review changes
  • Loading branch information
apb7 authored and kevinlee12 committed Jul 16, 2018
1 parent 6e4f5dc commit 7202b1e
Show file tree
Hide file tree
Showing 68 changed files with 627 additions and 431 deletions.
20 changes: 10 additions & 10 deletions core/controllers/admin_test.py
Expand Up @@ -73,7 +73,7 @@ def test_change_configuration_property(self):
self.UNICODE_TEST_STRING),
}
}
self.post_json('/adminhandler', payload, csrf_token)
self.post_json('/adminhandler', payload, csrf_token=csrf_token)

response_dict = self.get_json('/adminhandler')
response_config_properties = response_dict['config_properties']
Expand All @@ -99,7 +99,7 @@ def test_change_about_page_config_property(self):
'new_config_property_values': {
base.BEFORE_END_HEAD_TAG_HOOK.name: new_config_value
}
}, csrf_token)
}, csrf_token=csrf_token)

response = self.testapp.get('/about')
self.assertIn(new_config_value, response.body)
Expand All @@ -118,7 +118,7 @@ def test_generate_count_greater_than_publish_count(self):
'action': 'generate_dummy_explorations',
'num_dummy_exps_to_generate': 10,
'num_dummy_exps_to_publish': 3
}, csrf_token)
}, csrf_token=csrf_token)
generated_exps = exp_services.get_all_exploration_summaries()
published_exps = exp_services.get_recently_published_exp_summaries(5)
self.assertEqual(len(generated_exps), 10)
Expand All @@ -134,7 +134,7 @@ def test_generate_count_equal_to_publish_count(self):
'action': 'generate_dummy_explorations',
'num_dummy_exps_to_generate': 2,
'num_dummy_exps_to_publish': 2
}, csrf_token)
}, csrf_token=csrf_token)
generated_exps = exp_services.get_all_exploration_summaries()
published_exps = exp_services.get_recently_published_exp_summaries(5)
self.assertEqual(len(generated_exps), 2)
Expand Down Expand Up @@ -180,7 +180,7 @@ def test_view_and_update_role(self):
# Check normal user has expected role. Viewing by username.
response_dict = self.get_json(
feconf.ADMIN_ROLE_HANDLER_URL,
{'method': 'username', 'username': 'user1'})
params={'method': 'username', 'username': 'user1'})
self.assertEqual(
response_dict, {'user1': feconf.ROLE_ID_EXPLORATION_EDITOR})

Expand All @@ -197,7 +197,7 @@ def test_view_and_update_role(self):
# Viewing by role.
response_dict = self.get_json(
feconf.ADMIN_ROLE_HANDLER_URL,
{'method': 'role', 'role': feconf.ROLE_ID_MODERATOR})
params={'method': 'role', 'role': feconf.ROLE_ID_MODERATOR})
self.assertEqual(response_dict, {'user1': feconf.ROLE_ID_MODERATOR})
self.logout()

Expand All @@ -209,7 +209,7 @@ def test_invalid_username_in_view_and_update_role(self):
# Trying to view role of non-existent user.
response = self.get_json(
feconf.ADMIN_ROLE_HANDLER_URL,
{'method': 'username', 'username': username},
params={'method': 'username', 'username': username},
expected_status_int=400, expect_errors=True)

# Trying to update role of non-existent user.
Expand Down Expand Up @@ -264,7 +264,7 @@ def test_data_extraction_handler(self):
}

response = self.get_json(
'/explorationdataextractionhandler', payload)
'/explorationdataextractionhandler', params=payload)
extracted_answers = response['data']
self.assertEqual(len(extracted_answers), 2)
self.assertEqual(extracted_answers[0]['answer'], 'first answer')
Expand All @@ -279,7 +279,7 @@ def test_data_extraction_handler(self):
}

response = self.get_json(
'/explorationdataextractionhandler', payload)
'/explorationdataextractionhandler', params=payload)
extracted_answers = response['data']
self.assertEqual(len(extracted_answers), 1)
self.assertEqual(extracted_answers[0]['answer'], 'first answer')
Expand All @@ -294,7 +294,7 @@ def test_that_handler_raises_exception(self):
}

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

self.assertEqual(
Expand Down
2 changes: 1 addition & 1 deletion core/controllers/base.py
Expand Up @@ -217,7 +217,7 @@ def dispatch(self):
# If the request is to the old demo server, redirect it permanently to
# the new demo server.
if self.request.uri.startswith('https://oppiaserver.appspot.com'):
self.redirect('https://oppiatestserver.appspot.com', True)
self.redirect('https://oppiatestserver.appspot.com', permanent=True)
return

# In DEV_MODE, clearing cookies does not log out the user, so we
Expand Down
8 changes: 5 additions & 3 deletions core/controllers/base_test.py
Expand Up @@ -117,10 +117,12 @@ def test_requests_for_invalid_paths(self):
response = self.testapp.get('/library/data/extra', expect_errors=True)
self.assertEqual(response.status_int, 404)

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

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

def test_redirect_in_logged_out_states(self):
Expand Down Expand Up @@ -292,7 +294,7 @@ def test_jinja_autoescaping(self):
self.assertNotIn('x153y', response.body)

def test_special_char_escaping(self):
response = self.testapp.post('/fake', {})
response = self.testapp.post('/fake', params={})
self.assertEqual(response.status_int, 200)

self.assertTrue(response.body.startswith(feconf.XSSI_PREFIX))
Expand Down
3 changes: 2 additions & 1 deletion core/controllers/classifier.py
Expand Up @@ -38,7 +38,8 @@ def generate_signature(secret, message):
str. The signature of the payload data.
"""
message_json = json.dumps(message, sort_keys=True)
return hmac.new(secret, message_json, digestmod=hashlib.sha256).hexdigest()
return hmac.new(
secret, msg=message_json, digestmod=hashlib.sha256).hexdigest()


def validate_job_result_message_dict(message):
Expand Down
10 changes: 6 additions & 4 deletions core/controllers/collection_editor_test.py
Expand Up @@ -188,7 +188,8 @@ def test_editable_collection_handler_put_can_access(self):
def test_collection_rights_handler(self):
collection_id = 'collection_id'
collection = collection_domain.Collection.create_default_collection(
collection_id, 'A title', 'A Category', 'An Objective')
collection_id, title='A title',
category='A Category', objective='An Objective')
collection_services.save_new_collection(self.owner_id, collection)

# Check that collection is published correctly.
Expand Down Expand Up @@ -221,7 +222,8 @@ def test_get_collection_rights(self):

collection_id = 'collection_id'
collection = collection_domain.Collection.create_default_collection(
collection_id, 'A title', 'A Category', 'An Objective')
collection_id, title='A title',
category='A Category', objective='An Objective')
collection_services.save_new_collection(self.owner_id, collection)

# Check that collection is published correctly.
Expand Down Expand Up @@ -256,7 +258,7 @@ def test_publish_unpublish_collection(self):
response_dict = self.put_json(
'/collection_editor_handler/publish/%s' % collection_id,
{'version': collection.version},
csrf_token)
csrf_token=csrf_token)
self.assertFalse(response_dict['is_private'])
self.logout()

Expand All @@ -268,6 +270,6 @@ def test_publish_unpublish_collection(self):
response_dict = self.put_json(
'/collection_editor_handler/unpublish/%s' % collection_id,
{'version': collection.version},
csrf_token)
csrf_token=csrf_token)
self.assertTrue(response_dict['is_private'])
self.logout()
2 changes: 1 addition & 1 deletion core/controllers/creator_dashboard_test.py
Expand Up @@ -702,7 +702,7 @@ def test_new_exploration_ids(self):
self.assertEqual(response.status_int, 200)
csrf_token = self.get_csrf_token_from_response(response)
exp_a_id = self.post_json(
feconf.NEW_EXPLORATION_URL, {}, csrf_token
feconf.NEW_EXPLORATION_URL, {}, csrf_token=csrf_token
)[creator_dashboard.EXPLORATION_ID_KEY]
self.assertEqual(len(exp_a_id), 12)

Expand Down
3 changes: 2 additions & 1 deletion core/controllers/editor.py
Expand Up @@ -490,7 +490,8 @@ def get(self, exploration_id):
self.response.headers['Content-Disposition'] = (
'attachment; filename=%s.zip' % str(filename))
self.response.write(
exp_services.export_to_zip_file(exploration_id, version))
exp_services.export_to_zip_file(
exploration_id, version=version))
elif output_format == feconf.OUTPUT_FORMAT_JSON:
self.render_json(exp_services.export_states_to_yaml(
exploration_id, version=version, width=width))
Expand Down

0 comments on commit 7202b1e

Please sign in to comment.