Issue 4814 - Adds csv log to formmail2

sitescripts/formmail/test/test_formmail2.py

 # This file is part of the Adblock Plus web scripts,
-# Copyright (C) 2006-2016 Eyeo GmbH
+# Copyright (C) 2006-2017 eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # 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/>.
-import os
 
 from urllib import urlencode
 from urllib2 import urlopen, HTTPError
+from csv import DictReader
 
 import pytest
-import datetime
 from wsgi_intercept import (urllib_intercept, add_wsgi_intercept,
                             remove_wsgi_intercept)
-from csv import DictReader
+
 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']

[Vasily Kuznetsov, 2017/02/08 18:16:17]

As discussed over TS, let's override the csv_log with a temporary path here.

[Jon Sonesen, 2017/02/10 13:57:00]

Done.

 
 
-@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 log_path(form_config, tmpdir_factory):
-    log_file = form_config['csv_log'].value
-    log = tmpdir_factory.mktemp(log_file).join(log_file)
-    return str(log)
+@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',
     }
 
 
-@pytest.fixture()
-def response_for(form_handler, log_path, monkeypatch):
-    # override the configured test.csv path
-    monkeypatch.chdir(os.path.dirname(log_path))

[Vasily Kuznetsov, 2017/02/08 18:16:17]

As discussed over TS, we won't need this if override the `log_path` in
`form_config`.

[Jon Sonesen, 2017/02/10 13:56:59]

Done.

-    host, port = 'test.local', 80
+@pytest.fixture
+def response_for(form_handler):
+    """ 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()
 
 
+@pytest.fixture
+def sm_mock(mocker):
+    return mocker.patch('sitescripts.formmail.web.formmail2.sendMail')
+
+
+@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', form_config)[1]
+    assert error.value.message == message
+
+
+@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()
+        # rows should not be equal because the time field
+        # is added by the logging function.
+        assert row != reader.next()
+
+
+def test_config_field_errors(form_config):
+    form_config['fields'] = {}
+    with pytest.raises(Exception) as error:
+        formmail2.make_handler('test', form_config)[1]
+    assert error.value.message == 'No fields configured for form handler: test'
+
+
+def test_config_template_errors(form_config):
+    form_config['template'].value = 'no'
+    with pytest.raises(Exception) as error:
+        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):
+        response_for(form_data, log=True)
+
+    with open(log_path) as log_file:
+        row = DictReader(log_file).next()
+        assert row != form_data
 
 
-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, '')
+def test_append_field_err(form_config, form_data, log_path):
+    """ Checks that error logs are correctly written and appended
 
+    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']
 
-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'
+    # submit two forms with fields that dont match the config
+    # this should append the second form to the error log file
+    with pytest.raises(Exception):
+        formmail2.log_formdata(form_data, log_path)
+    with pytest.raises(Exception):
+        formmail2.log_formdata(form_data, log_path)
 
-
-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'
-
-    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'
-
-
-def test_collect_with_tmpl(log_path, form_data):
-    form_data['time'] = 'test'
-    formmail2.collect_formdata('test', form_data, log_path)
-    with open(log_path) as csvfile:
-        reader = DictReader(csvfile)
-        row = reader.next()
-        assert row == form_data
-
-
-def test_collect_no_tmpl(log_path, form_data, form_config):
-    del(form_config['template'])
-    form_data['time'] = 'test'
-    formmail2.collect_formdata('test', form_data, log_path)
-    with open(log_path) as csvfile:
-        reader = DictReader(csvfile)
-        row = reader.next()
-        assert row == form_data
-
-
-def test_fieldnames(log_path, form_data):
-    form_data['time'] = str(datetime.datetime.now())
-    formmail2.collect_formdata('test', form_data, log_path)
-    with open(log_path) as csvfile:
-        reader = DictReader(csvfile)
-        for field in reader.fieldnames:
-            assert field in tuple(form_data.keys())
-
-

[Vasily Kuznetsov, 2017/02/08 18:16:16]

We should also have a test for the situation when the configuration changes and
the field list is not the same as in the log file. Then it should give an error
and write to a temp file next to the log.

It could be tricky to set this up, but I think if you just modify `form_config`
to change the list of fields and then give it to `formmail2.make_handler` to
make another handler, it will work. Since we don't need to reuse this modified
config and handler they don't need to be fixtures, we just do it inside this one
test.

[Jon Sonesen, 2017/02/10 13:57:00]

Great

-def test_append_log(form_data, log_path):
-    """
-    collect data twice, altering a field in the second call
-    assert that the 2nd row is equal to the resulting form data
-    """
-    form_data['time'] = str(datetime.datetime.now())
-    formmail2.collect_formdata('test', form_data, log_path)
-    form_data['email'] = 'test@foo.com'
-    formmail2.collect_formdata('test', form_data, log_path)
-    with open(log_path) as csvfile:
-        reader = DictReader(csvfile, fieldnames=form_data.keys())
-        # header
-        reader.next()
-        row1 = reader.next()
-        row2 = reader.next()
-
-        assert row2['email'] == form_data['email']
-        assert row2['email'] != row1['email']
+    with open(log_path + '_error') as error_log:
+        reader = DictReader(error_log)
+        assert reader.next() == form_data
+        assert reader.next() == form_data