Issue 7059 - Modify fldiff so it can handle multiple files

tests/test_diff_script.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 the diff script."""
 
 from __future__ import unicode_literals
 
 import pytest
 import subprocess
 import io
+import os
+import re
 
 from test_differ import BASE, LATEST
+from abp.filters.diff_script import main as diff_script
 
 
 @pytest.fixture
 def rootdir(tmpdir):
     """Directory with example filter lists."""
     rootdir = tmpdir.join('root')
     rootdir.mkdir()
-    rootdir.join('base.txt').write_text(BASE, encoding='utf8')
     rootdir.join('latest.txt').write_text(LATEST, encoding='utf8')
-
     return rootdir
 
 
 @pytest.fixture
-def dstfile(tmpdir):
-    """Destination file for saving the diff output."""
-    return tmpdir.join('dst')
+def archive_dir(rootdir):
+    return rootdir.mkdir('archive')
+
+
+@pytest.fixture
+def diff_dir(rootdir):
+    return rootdir.mkdir('diff')
+
+
+@pytest.fixture
+def archived_files(archive_dir):
+    base2 = BASE + '&adnet=\n'
+    base2 = re.sub(r'! Version: \d+', '! Version: 112', base2)
+    archive_dir.join('list111.txt').write_text(BASE, encoding='utf8')
+    archive_dir.join('list112.txt').write_text(base2, encoding='utf8')
+    return [str(x) for x in archive_dir.listdir()]
+
+
+@pytest.fixture
+def base_no_version(archive_dir):
+    base = re.sub(r'! Version: \d+', '! ', BASE)
+    archive_dir.join('list113.txt').write_text(base, encoding='utf8')
+    return [str(x) for x in archive_dir.listdir()]
 
 
 def run_script(*args, **kw):
     """Run diff rendering script with given arguments and return its output."""
     cmd = ['fldiff'] + list(args)
 
     proc = subprocess.Popen(cmd, stderr=subprocess.PIPE,
                             stdout=subprocess.PIPE, stdin=subprocess.PIPE,
                             **kw)
     stdout, stderr = proc.communicate()
     return proc.returncode, stderr.decode('utf-8'), stdout.decode('utf-8')
 
 
-def test_diff_with_outfile(rootdir, dstfile):
-    run_script(str(rootdir.join('base.txt')),
-               str(rootdir.join('latest.txt')),
-               str(dstfile))
-    with io.open(str(dstfile), encoding='utf-8') as dst:
-        result = dst.read()
-    assert '+ &ad_channel=\xa3' in result
+def _make_args(rootdir, diff_dir):
+    return [str(rootdir.join('latest.txt')), '-o ' + str(diff_dir)]

[Vasily Kuznetsov, 2018/10/29 16:31:48]

'-o' and str(diff_dir) would be different arguments, normally, when the script
is called from a shell. It seems that this works anyway, but I think it would be
more realistic to replace it with ", '-o', str(diff_dir)]".

[rhowell, 2018/10/29 20:01:40]

Ended up just removing this function, but I did switch to this style in the
other functions.

[Vasily Kuznetsov, 2018/10/29 23:47:33]

Acknowledged.

 
 
-def test_no_outfile(rootdir):
-    _, _, out = run_script(str(rootdir.join('base.txt')),
-                           str(rootdir.join('latest.txt')))
-    assert '[Adblock Plus Diff]' in out
+def test_diff_with_dict(rootdir, archived_files, diff_dir):
+    arg = _make_args(rootdir, diff_dir)

[Vasily Kuznetsov, 2018/10/29 16:31:48]

I wonder if _make_args is actually worth it. Here if we don't call it, we get
something like:

    diff_script([str(rootdir.join('latest.txt')), '-o', str(diff_dir)]
                + archived_files)

Maybe it's even easier to understand. What do you think?

[rhowell, 2018/10/29 20:01:40]

Done.

+    arg.extend(archived_files)
+    diff_script(arg)
+    for root, dirs, files in os.walk(str(diff_dir)):
+        for file in files:
+            with io.open(str(diff_dir.join(file)), encoding='utf-8') as dst:
+                result = dst.read()
+            assert '- &ad.vid=$~xmlhttprequest' in result
+            assert '+ &ad_channel=\xa3' in result
+            assert '! Version: 123' in result
+
+
+def test_diff_with_outfile(rootdir, archived_files, diff_dir):
+    arg = _make_args(rootdir, diff_dir)
+    arg.extend(archived_files)
+    run_script(*arg)

[Vasily Kuznetsov, 2018/10/29 16:31:48]

Following on the previous comment, here we'd get something like:

    run_script(str(rootdir.join('latest.txt')), '-o', str(diff_dir),
               *archived_files)

What do you think about this version?

[rhowell, 2018/10/29 20:01:40]

Done.

+    for root, dirs, files in os.walk(str(diff_dir)):
+        assert files
+        for file in files:
+            with io.open(str(diff_dir.join(file)), encoding='utf-8') as dst:
+                result = dst.read()
+            assert '- &ad.vid=$~xmlhttprequest' in result
+            assert '+ &ad_channel=\xa3' in result
+            assert '! Version: 123' in result
 
 
 def test_no_base_file(rootdir):
-    code, err, _ = run_script('wrong.txt', str(rootdir.join('latest.txt')))
-    assert code == 1
-    assert 'No such file or directory' in err
+    code, err, _ = run_script(str(rootdir.join('latest.txt')))
+    assert code == 2
+    assert 'usage: fldiff' in err
 
 
-def test_no_latest_file(rootdir):
+def test_missing_file(rootdir):
     code, err, _ = run_script(str(rootdir.join('base.txt')), 'wrong.txt')
     assert code == 1
     assert 'No such file or directory' in err
 
 
-def test_diff_to_self(rootdir):
-    _, _, out = run_script(str(rootdir.join('latest.txt')),
-                           str(rootdir.join('latest.txt')))
-    assert out == '[Adblock Plus Diff]\n'
+def test_diff_to_self(rootdir, diff_dir):
+    arg = _make_args(rootdir, diff_dir)
+    arg.append(str(rootdir.join('latest.txt')))
+    run_script(*arg)
+    for root, dirs, files in os.walk(str(diff_dir)):
+        assert files
+        for file in files:
+            with io.open(str(diff_dir.join(file)), encoding='utf-8') as dst:
+                result = dst.read()
+            assert result == '[Adblock Plus Diff]\n'
+
+
+def test_no_version(rootdir, base_no_version):
+    arg = _make_args(rootdir, diff_dir)
+    arg.extend(base_no_version)
+    code, err, _ = run_script(*arg)
+    assert code == 1
+    assert err == 'Unable to find Version in [Adblock Plus 2.0]\n'