Issue 5333 - Allow cms to generate relative pages

tests/test_page_outputs.py

 import os
 import sys
 import runpy
 
 import mock
 import pytest
 
 from .conftest import ROOTPATH
 from .utils import get_dir_contents, exception_test
 from cms.sources import FileSource
 from cms.bin.test_server import DynamicServerHandler
 
 
-def get_expected_outputs(test_type):
-    expected_out_path = os.path.join(ROOTPATH, 'tests', 'expected_output')
-    outputs = get_dir_contents(expected_out_path)
-    for filename in list(outputs):
-        # Move test-type-specific expected outputs (e.g. "xyz@static" -> "xyz")
-        # There are cases where we need to test outputs which differ depending
-        # on how they are generated; either statically or dynamically
-        if filename.endswith('@' + test_type):
-            realname = filename.split('@')[0]
-            outputs[realname] = outputs[filename]
-        # Remove the expected outputs that don't apply for this test type.
-        if '@' in filename:
-            del outputs[filename]
+def get_expected_outputs(test_type, with_common=True):
+    root = os.path.join(ROOTPATH, 'tests', 'expected_output')
+    outputs = {}
+    if with_common:
+        outputs = get_dir_contents(os.path.join(root, 'common'))
+    outputs.update(get_dir_contents(os.path.join(root, test_type)))
     return outputs.items()
 
 
 static_expected_outputs = get_expected_outputs('static')
 dynamic_expected_outputs = get_expected_outputs('dynamic')
-relative_expected_outputs = get_expected_outputs('relative')
+relative_expected_outputs = get_expected_outputs('relative', False)
+
+
+def generate_static_pages(temp_site, tmpdir_factory, *extra_args):
+    dst_path = str(tmpdir_factory.mktemp('static'))
+    sys.argv = ['filler', temp_site, dst_path] + list(extra_args)
+    with mock.patch('cms.sources.FileSource.version', 1):
+        runpy.run_module('cms.bin.generate_static_pages',
+                         run_name='__main__')
+    return get_dir_contents(dst_path)
 
 
 @pytest.fixture(scope='session')
-def static_output(request, temp_site):
-    static_out_path = os.path.join(temp_site, 'static_out')
-    sys.argv = ['filler', temp_site, static_out_path]
-    with mock.patch('cms.sources.FileSource.version', 1):
-        runpy.run_module('cms.bin.generate_static_pages', run_name='__main__')
-    return static_out_path
+def output_pages(temp_site, tmpdir_factory):
+    return generate_static_pages(temp_site, tmpdir_factory)
 
 
 @pytest.fixture(scope='session')
-def static_output_relative(request, temp_site):

[Vasily Kuznetsov, 2018/11/02 10:45:35]

This code repetition is not that great. The different from `static_output` is
really small, it seems like they want to be the same function with just a
parameter.

This got me thinking: why did we even need this `static_output` fixture anyway,
it's only used by `output_pages` and that one doesn't have any real action. We
can replace the four fixtures with one function and two fixtures like this:

    def generate_static_pages(temp_site, tmpdir_factory, *extra_args):
        dst_path = str(tmpdir_factory.mktemp('static'))
        sys.argv = ['filler', temp_site, dst_path] + list(extra_args)
        with mock.patch('cms.sources.FileSource.version', 1):
            runpy.run_module('cms.bin.generate_static_pages',
run_name='__main__')
        return get_dir_contents(dst_path)


    @pytest.fixture(scope='session')
    def output_pages(temp_site, tmpdir_factory):
        return generate_static_pages(temp_site, tmpdir_factory)


    @pytest.fixture(scope='session')
    def output_pages_relative(temp_site, tmpdir_factory):
        return generate_static_pages(temp_site, tmpdir_factory, '--relative')

What do you think?

[rhowell, 2018/11/06 03:34:41]

Yeah, good idea. This seems much cleaner (and more extendable in the future).

-    static_out_path = os.path.join(temp_site, 'static_out_relative')
-    sys.argv = ['filler', temp_site, static_out_path, '--relative']
-    with mock.patch('cms.sources.FileSource.version', 1):
-        runpy.run_module('cms.bin.generate_static_pages', run_name='__main__')
-    return static_out_path
-
-
-@pytest.fixture(scope='session')
-def output_pages(static_output):
-    return get_dir_contents(static_output)
-
-
-@pytest.fixture(scope='session')
-def output_pages_relative(static_output_relative):
-    return get_dir_contents(static_output_relative)
+def output_pages_relative(temp_site, tmpdir_factory):
+    return generate_static_pages(temp_site, tmpdir_factory, '--relative')
 
 
 @pytest.mark.parametrize('filename,expected_output', static_expected_outputs)
 def test_static(output_pages, filename, expected_output):
     if expected_output.startswith('## MISSING'):
         assert filename not in output_pages
     else:
         assert expected_output == output_pages[filename]
 
 
 @pytest.mark.parametrize('filename,expected_output', relative_expected_outputs)

[Vasily Kuznetsov, 2018/11/02 10:45:35]

This function is also basically a clone of `test_static` but I don't see a
simple and elegant way to avoid duplication. Perhaps let's leave it like this
for now. I have one idea but it's kind of tricky and probably should be a
separate ticket.

[rhowell, 2018/11/06 03:34:42]

Acknowledged.

 def test_static_relative(output_pages_relative, filename, expected_output):
     if expected_output.startswith('## MISSING'):
         assert filename not in output_pages_relative
     else:
         assert expected_output == output_pages_relative[filename]
 
 
 def test_cache(output_pages):
     source = FileSource(os.path.join('test_site'))
     assert source.get_cache_dir() == os.path.join('test_site', 'cache')
 
 
 @pytest.mark.parametrize('filename,expected_output', dynamic_expected_outputs)
 def test_dynamic_server_handler(filename, expected_output, temp_site):
+
     def cleanup(page):
         return page.replace(os.linesep, '').strip()
 
     handler = DynamicServerHandler('localhost', 5000, str(temp_site))
     environ = {'PATH_INFO': filename}
 
     generated_page = handler(environ, lambda x, y: None)
 
     assert cleanup(expected_output) == cleanup(generated_page[0])
 
 
 @pytest.mark.parametrize('page', ['en/translate', '/en/translate'])
 def test_dynamic_server_handler_with_conflicts(page, temp_site_with_conflicts):
     handler = DynamicServerHandler('localhost', 5000,
                                    str(temp_site_with_conflicts))
     environ = {'PATH_INFO': page}
     exp_msg = 'The requested page conflicts with another page.'
 
     exception_test(handler, Exception, exp_msg, environ, lambda x, y: None)