Issue 7230 - python-abp line2dict vectorization

tests/test_rpy.py

 # This file is part of Adblock Plus <https://adblockplus.org/>,
 # Copyright (C) 2006-present 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/>.
 
 """Functional tests for testing rPython integration."""
 from __future__ import unicode_literals
 
 from collections import namedtuple
 import pytest
 import sys
 
-from abp.filters.rpy import tuple2dict, line2dict
+from abp.filters.rpy import tuple2dict, line2dict, lines2dicts
 
 
 _SAMPLE_TUPLE = namedtuple('tuple', 'foo,bar')
 
 _TEST_EXAMPLES = {
     'header': {
         'in': b'[Adblock Plus 2.0]',
         'out': {
             b'type': b'Header',
             b'version': b'Adblock Plus 2.0',
@@ skipping 106 matching lines @@
 @pytest.mark.parametrize('line_type', list(_TEST_EXAMPLES.keys()))
 def test_line2dict_format(line_type):
     """Test that the API result has the appropriate format.
 
     Checks for both keys and datatypes.
     """
     position = 'start' if line_type in {'header', 'metadata'} else 'body'
     data = line2dict(_TEST_EXAMPLES[line_type]['in'], position)
 
     assert data == _TEST_EXAMPLES[line_type]['out']
+
+
+def test_lines2dicts():
+    """Test that the API result has the appropriate format.
+
+    Checks for both keys and datatypes.
+    """
+    string_list = []
+    line_types = {b'EmptyLine': 'empty', b'Filter': 'filter_single',
+                  b'Comment': 'comment', b'Include': 'include'}
+
+    for key in _TEST_EXAMPLES.keys():

[Vasily Kuznetsov, 2019/01/25 15:00:58]

This is equivalent to `for key in _TEST_EXAMPLES:`. I would recommend to either
use that and keep the rest of the code as is, or use the idiom:

    for key, value in _TEST_EXAMPLES.items():

This is a bit more "pythonic".

[rhowell, 2019/01/26 01:08:22]

Done.

+        if (_TEST_EXAMPLES[key]['out'][b'type'] != b'Header'

[Vasily Kuznetsov, 2019/01/25 15:00:58]

You can use `... not in {b'Header', b'Metadata'}`.

[rhowell, 2019/01/26 01:08:22]

Done.

+                and _TEST_EXAMPLES[key]['out'][b'type'] != b'Metadata'):
+            string_list.append(_TEST_EXAMPLES[key]['in'])
+
+    result_list = lines2dicts(string_list)
+
+    for line in result_list:
+        line_type = line_types[line[b'type']]
+        data = line2dict(_TEST_EXAMPLES[line_type]['in'], 'start')
+
+        assert data == _TEST_EXAMPLES[line_type]['out']

[Vasily Kuznetsov, 2019/01/25 15:00:58]

So in the end we compare the output of
line2dict(_TEST_EXAMPLES[line_type]['in']) to _TEST_EXAMPLES[line_type]['out']
-- this seems like a test of line2dict with an innovative way of looping over
some part of _TEST_EXAMPLES. But we are not doing much checking of the output of
lines2dicts().

A better approach would be to first produce the list of all inputs and outputs:

    tests = [t for t in _TEST_EXAMPLES.values()
             if t[b'type'] not in {b'Header', b'Metadata'}]
    ins = [t['in'] for ex in tests]
    outs = [t['out'] for ex in tests]

and then we could just compare the output of `lines2dicts(ins)` to `outs`.

    assert lines2dicts(string_list, 'start') 

[rhowell, 2019/01/26 01:08:22]

Good idea, this test makes much more sense. I was trying to avoid something like
this because Python2 doesn't preserve the order of dicts, so the resulting lists
have the dicts in different orders, but the tests are passing every time on
Python2.7 so now I'm confused. Normally, lists that are in different order are
not considered equal.

E.g.

--> 179     assert lines2dicts(ins) == outs

ipdb> lines2dicts(ins)
[{'target': 'www.test.py/filtelist.txt', 'type': 'Include'}, {'selector':
{'type': 'css', 'value': 'div#ad1'}, 'action': 'hide', 'options': [('domain',
[('foo.com', True), ('bar.com', True)])], 'text': 'foo.com,bar.com##div#ad1',
'type': 'Filter'}, {'type': 'EmptyLine'}, {'type': 'Comment', 'text':
'Comment'}, {'selector': {'type': 'css', 'value': 'div#ad1'}, 'action': 'hide',
'options': [('domain', [('foo.com', True)])], 'text': 'foo.com##div#ad1',
'type': 'Filter'}]

ipdb> outs
[{'target': 'www.test.py/filtelist.txt', 'type': 'Include'}, {'selector':
{'value': 'div#ad1', 'type': 'css'}, 'action': 'hide', 'type': 'Filter',
'options': [('domain', [('foo.com', True), ('bar.com', True)])], 'text':
'foo.com,bar.com##div#ad1'}, {'type': 'EmptyLine'}, {'type': 'Comment', 'text':
'Comment'}, {'selector': {'value': 'div#ad1', 'type': 'css'}, 'action': 'hide',
'type': 'Filter', 'options': [('domain', [('foo.com', True)])], 'text':
'foo.com##div#ad1'}]

ipdb> lines2dicts(ins) == outs
True

But:

$ python2
Python 2.7.12 (default, Nov 12 2018, 14:36:49) 
[GCC 5.4.0 20160609] on linux2
False

[Vasily Kuznetsov, 2019/01/28 17:47:36]

This happens because the proposed code doesn't depend on the order of iteration
over a dict. The first line picks the tests that we are going to run and saves
them in a python list (`tests`). This way the order stays the same when we use
it to produce the lists of inputs and outputs.

[rhowell, 2019/01/29 01:44:47]

Ah, right, makes sense. I ran into this issue when I thought I read
`lines2dict`, and it was returning a dict. But now that it returns a list, the
order it preserved. Got it, thanks for the clarification!