Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: sitescripts/formmail/test/test_formmail2.py

Issue 29374647: Issue 4814 - Adds csv log to formmail2 (Closed) Base URL: https://hg.adblockplus.org/sitescripts
Patch Set: refactor tests to be parametrized, reducing duplication. Addressed some redundancies Created March 22, 2017, 8:14 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « .sitescripts.example ('k') | sitescripts/formmail/web/formmail2.py » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: sitescripts/formmail/test/test_formmail2.py
===================================================================
--- a/sitescripts/formmail/test/test_formmail2.py
+++ b/sitescripts/formmail/test/test_formmail2.py
@@ -7,144 +7,195 @@
#
# Adblock Plus is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.
+
from urllib import urlencode
from urllib2 import urlopen, HTTPError
+from csv import DictReader
import pytest
from wsgi_intercept import (urllib_intercept, add_wsgi_intercept,
remove_wsgi_intercept)
from sitescripts.formmail.web import formmail2
+HOST = 'test.local'
+LOG_PORT = 80
+NO_LOG_PORT = 81
-@pytest.fixture()
-def form_config():
+
+@pytest.fixture
+def log_path(tmpdir):
+ return str(tmpdir.join('test.csv_log'))
+
+
+@pytest.fixture
+def log_form_config():
return formmail2.conf_parse(formmail2.get_config_items())['test']
-@pytest.fixture()
-def form_handler(form_config):
- return formmail2.make_handler('test', form_config)[1]
+@pytest.fixture
+def form_config():
+ config = formmail2.conf_parse(formmail2.get_config_items())['test']
+ del config['csv_log']
+ return config
+
+
+@pytest.fixture
+def form_handler(log_path, form_config, log_form_config):
+ """ Create two handlers, one that logs and another that doesn't """
+ log_form_config['csv_log'].value = log_path
+ return (formmail2.make_handler('test', log_form_config)[1],
+ formmail2.make_handler('test', form_config)[1])
# We make this a fixture instead of a constant so we can modify it in each
# test as needed without affecting other tests.
@pytest.fixture
def form_data():
return {
'email': 'john_doe@gmail.com',
'mandatory': 'john_doe@gmail.com',
'non_mandatory_message': 'Once upon a time\nthere lived a king.',
- 'non_mandatory_email': 'test@test.com'
+ 'non_mandatory_email': 'test@test.com',
}
-@pytest.fixture()
+@pytest.fixture
def response_for(form_handler):
- host, port = 'test.local', 80
+ """ Registers two intercepts, returns responses for them based on bool """
urllib_intercept.install_opener()
- add_wsgi_intercept(host, port, lambda: form_handler)
- url = 'http://{}:{}'.format(host, port)
+ add_wsgi_intercept(HOST, LOG_PORT, lambda: form_handler[0])
+ add_wsgi_intercept(HOST, NO_LOG_PORT, lambda: form_handler[1])
- def response_for(data):
+ def response_for(data, log=False):
+ if log:
+ url = 'http://{}:{}'.format(HOST, LOG_PORT)
+ else:
+ url = 'http://{}:{}'.format(HOST, NO_LOG_PORT)
if data is None:
response = urlopen(url)
else:
response = urlopen(url, urlencode(data))
return response.code, response.read()
yield response_for
remove_wsgi_intercept()
-def test_form_handler_email_errors(form_config):
- tmp_config = form_config
- del tmp_config['url'].value
- with pytest.raises(Exception) as error:
- formmail2.make_handler('test', tmp_config)[1]
- assert error.value.message == 'No URL configured for form handler: test'
+@pytest.fixture
+def sm_mock(mocker):
+ return mocker.patch('sitescripts.formmail.web.formmail2.sendMail')
-def test_form_handler_field_errors(form_config):
- tmp_config = form_config
- tmp_config['fields'] = {}
+@pytest.mark.parametrize('key,message', [
+ ('url', 'No URL configured for form handler: test'),
+ ('fields', 'No fields configured for form handler: test'),
+ ('template', 'No template configured for form handler: test'),
+ ])
+def test_config_errors(key, message, form_config):
+ del form_config[key]
with pytest.raises(Exception) as error:
- formmail2.make_handler('test', tmp_config)[1]
- assert error.value.message == 'No fields configured for form handler: test'
+ formmail2.make_handler('test', form_config)[1]
+ assert error.value.message == message
- del tmp_config['fields']
+
+@pytest.mark.parametrize('field,message', [
+ (('new_field', 'foo'), 'Unexpected field/fields: new_field'),
+ (('mandatory', ''), 'No mandatory entered'),
+ (('non_mandatory_email', 'asfaf'), 'Invalid email'),
+ (('email', 'asfaf'), 'You failed the email validation'),
+ (('email', ''), 'You failed the email test'),
+ ])
+def test_http_errs(field, message, response_for, form_data, sm_mock):
+ key, value = field
+ form_data[key] = value
+ with pytest.raises(HTTPError) as error:
+ response_for(form_data)
+ assert error.value.read() == message
+
+
+@pytest.mark.parametrize('field,expected', [
+ (('non_mandatory_message', '\xc3\xb6'), (200, '')),
+ (('non_mandatory_message', ''), (200, '')),
+ ])
+def test_success(field, expected, log_path, response_for, form_data, sm_mock):
+ key, value = field
+ form_data[key] = value
+ assert response_for(form_data, log=False) == expected
+ assert sm_mock.call_count == 1
+
+ params = sm_mock.call_args[0][1]['fields']
+ assert set(params.keys()) == set(form_data.keys())
+ for key, value in form_data.items():
+ assert params[key] == value.decode('utf8')
+
+ assert response_for(form_data, log=True) == expected
+ assert sm_mock.call_count == 2
+
+ assert response_for(form_data, log=True) == expected
+ assert sm_mock.call_count == 3
+
+ with open(log_path) as log_file:
+ reader = DictReader(log_file)
+ row = reader.next()
+ assert row != reader.next()
Vasily Kuznetsov 2017/03/23 13:19:16 Are they not equal because of time field? What do
Jon Sonesen 2017/03/23 13:31:01 Done.
+
+
+def test_config_field_errors(form_config):
+ form_config['fields'] = {}
with pytest.raises(Exception) as error:
- formmail2.make_handler('test', tmp_config)[1]
+ formmail2.make_handler('test', form_config)[1]
assert error.value.message == 'No fields configured for form handler: test'
-def test_form_handler_template_errors(form_config):
- tmp_config = form_config
- tmp_config['template'].value = 'no'
+def test_config_template_errors(form_config):
+ form_config['template'].value = 'no'
with pytest.raises(Exception) as error:
- formmail2.make_handler('test', tmp_config)[1]
- assert error.typename == 'TemplateNotFound'
-
- del tmp_config['template'].value
- with pytest.raises(Exception) as error:
- formmail2.make_handler('test', tmp_config)[1]
- assert error.value.message == ('No template configured for form handler'
- ': test')
- del tmp_config['template']
- with pytest.raises(Exception) as error:
- formmail2.make_handler('test', tmp_config)[1]
- assert error.value.message == ('No template configured for form handler'
- ': test')
+ formmail2.make_handler('test', form_config)[1]
+ assert error.value.message == 'Template not found at: no'
def test_config_parse(form_config):
assert form_config['url'].value == 'test/apply/submit'
assert form_config['fields']['email'].value == 'mandatory, email'
-def test_success(response_for, form_data, mocker):
- sm_mock = mocker.patch('sitescripts.formmail.web.formmail2.sendMail')
- assert response_for(form_data) == (200, '')
- assert sm_mock.call_count == 1
- params = sm_mock.call_args[0][1]['fields']
- assert set(params.keys()) == set(form_data.keys())
- for key, value in form_data.items():
- assert params[key] == value
+def test_sendmail_fail(log_path, response_for, form_data, sm_mock):
+ sm_mock.side_effect = Exception('Sendmail Fail')
+ with pytest.raises(HTTPError) as error:
+ response_for(form_data, log=True)
+ assert error.typename == 'HTTPError'
Vasily Kuznetsov 2017/03/23 13:19:16 You probably don't need to assert this since you'v
Jon Sonesen 2017/03/23 13:31:01 Acknowledged.
-
-def test_non_mandatory_no_msg(response_for, form_data, mocker):
- mocker.patch('sitescripts.formmail.web.formmail2.sendMail')
- form_data['non_mandatory'] = ''
- assert response_for(form_data) == (200, '')
+ with open(log_path) as log_file:
+ row = DictReader(log_file).next()
+ assert 'time' in row
-def test_invalid_email_cstm_msg(response_for, form_data, mocker, form_config):
- mocker.patch('sitescripts.formmail.web.formmail2.sendMail')
- form_data['email'] = 'bademail'
- with pytest.raises(HTTPError) as error:
- response_for(form_data)
- assert error.value.read() == 'You failed the email validation'
-
+def test_append_field_err(form_config, form_data, log_path):
+ """ Checks that error logs are correctly written and appended
-def test_valid_nan_mandatory_email(response_for, form_data, mocker):
- mocker.patch('sitescripts.formmail.web.formmail2.sendMail')
- form_data['non_mandatory_email'] = 'asfaf'
- with pytest.raises(HTTPError) as error:
- response_for(form_data)
- assert error.value.read() == 'Invalid email'
+ Submits three forms, the second two have different fields to the first
+ and should be added to the same log file as each other, and be identical
+ """
+ formmail2.log_formdata(form_data, log_path)
+ del(form_data['email'])
+ try:
+ formmail2.log_formdata(form_data, log_path)
+ except Exception as e:
Vasily Kuznetsov 2017/03/23 13:19:16 Perhaps it's better to use with pytest.raises(...)
Jon Sonesen 2017/03/23 13:31:01 Done. Odd that I chose to use try's in the first p
+ assert e.message == ('Field names have changed, error log '
+ 'written to {}_error').format(log_path)
+ try:
Vasily Kuznetsov 2017/03/23 13:19:16 I suppose there's some reason for trying this twic
Jon Sonesen 2017/03/23 13:31:01 Done.
+ formmail2.log_formdata(form_data, log_path)
+ except Exception as e:
+ assert e.message == ('Field names have changed, error log'
Vasily Kuznetsov 2017/03/23 13:19:16 Do we need to assert the whole message here again
Jon Sonesen 2017/03/23 13:31:01 Agreed, I will remove the assertions
+ ' appended to {}_error').format(log_path)
- del form_data['non_mandatory_email']
- assert response_for(form_data) == (200, '')
-
-
-def test_mandatory_fail_dflt_msg(response_for, form_data, mocker):
- mocker.patch('sitescripts.formmail.web.formmail2.sendMail')
- del form_data['mandatory']
- with pytest.raises(HTTPError) as error:
- response_for(form_data)
- assert error.value.read() == 'No mandatory entered'
+ with open(log_path + '_error') as error_log:
+ reader = DictReader(error_log)
+ assert reader.next() == form_data
+ assert reader.next() == form_data
« no previous file with comments | « .sitescripts.example ('k') | sitescripts/formmail/web/formmail2.py » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld