Issue 5934 - CMS testing automation

cms-dev/tests/test_cms_cmp.py

 # This file is part of Adblock Plus <https://adblockplus.org/>,
 # Copyright (C) 2017-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/>.
 
 import glob
 import os
 import subprocess
 import sys
 
 import pytest
 
 CMSCMP = os.path.join(os.path.dirname(os.path.dirname(__file__)), 'cms_cmp.py')
 
 
 def run_cms_cmp(*args, **kw):
     return subprocess.check_call([sys.executable, CMSCMP] + list(args), **kw)
 
 
 def hg(*args, **kw):

[tlucas, 2017/11/24 10:57:46]

This is a copy of cms_cmp.hg(), without the call of run_cmd() - is there any
specific reason, why you are not simply importing it?

Imho the tests would also benefit from the beautified error message you print in
cmp_cms.run_cmd().

[Vasily Kuznetsov, 2017/11/28 16:53:37]

I had in mind three arguments for implementing a separate `hg()` function here:
  1. We don't want to use the code under test for running the tests.
  2. `run_cmd()` throws `SystemExit` exception, which normally leads to process
exit.
  3. We need to set `PYTHONPATH` in order to import `cms_cmp.py` and that's kind
of ugly.

However, (1) is not a very strong argument and (2) is based on a faulty
assumption (actually, pytest catches `SystemExit` and reports it  as a test
failure, so the behavior of tests, in which `hg` fails, would not change). (3)
is valid, but should instead be solved by making this a python package (which I
thought was a bit of an overkill).

All in all, I'm leaning towards following your advice as soon as this is made
into a Python package, but perhaps not right now. What do you think?

[tlucas, 2017/12/14 10:30:42]

I agree, right now this is not a very urgent topic and yes, it would be nice to
someday have this as a python package.

     cmd = ['hg']
     if 'repo' in kw:
         cmd += ['-R', kw['repo']]
         del kw['repo']
     # Disable default options from local user config.
     cmd += ['--config', 'defaults.{}='.format(args[0])]
     return subprocess.check_call(cmd + list(args), **kw)
 
 
 @pytest.fixture(scope='session')
@@ skipping 70 matching lines @@
     with pytest.raises(subprocess.CalledProcessError):
         run_cms_cmp('-d', str(tmpdir), '-c', str(cms), '-t', 'yoda',
                     str(website))
     # ...but it will succeed if we tell it to delete old outputs.
     run_cms_cmp('-d', str(tmpdir), '-c', str(cms), '-t', 'yoda', '-r',
                 str(website))
 
 
 def test_working_copy(website, cms, tmpdir):
     with pytest.raises(subprocess.CalledProcessError):
-        # This will fail because surrent version is 'other'.

[tlucas, 2017/11/24 10:57:46]

typo "current"

[Vasily Kuznetsov, 2017/11/28 16:53:37]

Done.

+        # This will fail because current version is 'other'.
         run_cms_cmp('-d', str(tmpdir), '-c', str(cms), str(website))
     generate = cms.join('cms').join('bin').join('generate_static_pages.py')
     # Remove last line that breaks stuff.
     generate.write('\n'.join(generate.read().splitlines()[:-1]))
     # Now it should be better.
     run_cms_cmp('-d', str(tmpdir), '-c', str(cms), str(website))