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

Issue 29350441: Noissue - Add tests for formmail handler (Closed)

Created:
Sept. 2, 2016, 2:35 p.m. by Vasily Kuznetsov
Modified:
Sept. 5, 2016, 9:52 a.m.
Reviewers:
Sebastian Noack
Visibility:
Public.

Description

Noissue - Add tests for formmail handler Repository: https://hg.adblockplus.org/sitescripts Base revision: 6465d6f74697

Patch Set 1 #

Total comments: 2

Patch Set 2 : Rearrange the imports to follow PEP8 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+97 lines, -1 line) Patch
A sitescripts/formmail/test/test_formmail.py View 1 1 chunk +93 lines, -0 lines 0 comments Download
M tox.ini View 2 chunks +4 lines, -1 line 0 comments Download

Messages

Total messages: 4
Vasily Kuznetsov
While looking at the existing form submission handler (in preparation for working on #4377) I ...
Sept. 2, 2016, 2:40 p.m. (2016-09-02 14:40:26 UTC) #1
Sebastian Noack
Looks mostly good. https://codereview.adblockplus.org/29350441/diff/29350442/sitescripts/formmail/test/test_formmail.py File sitescripts/formmail/test/test_formmail.py (right): https://codereview.adblockplus.org/29350441/diff/29350442/sitescripts/formmail/test/test_formmail.py#newcode16 sitescripts/formmail/test/test_formmail.py:16: import pytest corelib imports go first. ...
Sept. 2, 2016, 3:09 p.m. (2016-09-02 15:09:08 UTC) #2
Vasily Kuznetsov
Good point about imports order -- fixed it. https://codereview.adblockplus.org/29350441/diff/29350442/sitescripts/formmail/test/test_formmail.py File sitescripts/formmail/test/test_formmail.py (right): https://codereview.adblockplus.org/29350441/diff/29350442/sitescripts/formmail/test/test_formmail.py#newcode16 sitescripts/formmail/test/test_formmail.py:16: import ...
Sept. 2, 2016, 3:44 p.m. (2016-09-02 15:44:30 UTC) #3
Sebastian Noack
Sept. 2, 2016, 4:13 p.m. (2016-09-02 16:13:51 UTC) #4
LGTM

Powered by Google App Engine
This is Rietveld