Issue 4992 - Adds optional revision arg to generate_static_pages

tests/test_page_outputs.py

 import os
 import sys
 import time
 import runpy
 import signal
 import pytest
 import urllib2
 import subprocess
 from conftest import ROOTPATH
 
 
 def get_dir_contents(path):
     return_data = {}
     for dirpath, dirnames, filenames in os.walk(path):
         for output_file in filenames:
             with open(os.path.join(dirpath, output_file)) as f:
                 return_data[output_file] = f.read()
     return return_data
 
 
 def get_expected_outputs():
     expected_out_path = os.path.join(ROOTPATH, 'tests', 'expected_output')
     return get_dir_contents(expected_out_path).items()
 
 expected_outputs = get_expected_outputs()
 
 
+@pytest.fixture(scope='session', params=['master', None])
+def revision(request):
+    return request.param
+
+
 @pytest.fixture(scope='session')
-def static_output(request, temp_site):
+def static_output(revision, request, temp_site):
     static_out_path = os.path.join(temp_site, 'static_out')
-    sys.argv = ['filler', temp_site, static_out_path+'master',

[Vasily Kuznetsov, 2017/04/03 13:02:19]

I was actually thinking to only generate the master bookmark for
`test_revision_arg` and leave everything else as is. It seems like it would be
simpler than generating two websites. Or if you want to run all the tests on
both, maybe we could just use parametrize somehow?

[Jon Sonesen, 2017/04/03 13:32:26]

I am not sure how this would work then. The thing is that want to test that the
generation is done with the expected, provided revision. So unless we only
wanted to test that the argparse code works then wouldn't we have to generate
two sites to ensure they are not from the same revision/bookmark and to test
this wouldn't we need to add a file or some other change in the generated site?

Perhaps I am misunderstanding how you want to test the behavior.

[Jon Sonesen, 2017/04/03 13:49:26]

As discussed over Mumble we agree this bit of duplication in tests and
everything is not needed. Instead I will alter this method to accept a new
parametrized fixture called 'revision' which returns None, and 'master' then
based on this value we can assert the expected outputs.

-                '--rev', 'master']
+    sys.argv = ['filler', temp_site, static_out_path]
+    if revision is not None:
+        sys.argv += ['--rev', revision]
+
     runpy.run_module('cms.bin.generate_static_pages', run_name='__main__')
-    sys.argv = ['filler', temp_site, static_out_path+'default']
-    runpy.run_module('cms.bin.generate_static_pages', run_name='__main__')
-    return static_out_path + 'master', static_out_path + 'default'
+    return static_out_path
 
 
 @pytest.yield_fixture()
 def dynamic_server(temp_site):
     args = ['python', 'runserver.py', temp_site]
     # Werkzeug is a dependency of flask which we are using for the mock api
     # however there is an issue with Werkzeug that prevents it from properly
     # handling the SIGTERM sent by p.kill() or terminate()
     # Issue: https://github.com/pallets/werkzeug/issues/58
     p = subprocess.Popen(args, stdout=subprocess.PIPE, preexec_fn=os.setsid)
     time.sleep(0.5)
     yield 'http://localhost:5000/root/'
     os.killpg(os.getpgid(p.pid), signal.SIGTERM)
 
 
 @pytest.fixture(scope='session')
 def output_pages(static_output):
-    return (get_dir_contents(static_output[0]),
-            get_dir_contents(static_output[1]))
+    return get_dir_contents(static_output)
 
 
 @pytest.mark.parametrize('filename,expected_output', expected_outputs)
 def test_static(output_pages, filename, expected_output):
-    assert output_pages[0][filename] == expected_output
-    assert output_pages[1][filename] == expected_output
+    assert output_pages[filename] == expected_output
 
 
 @pytest.mark.parametrize('filename,expected_output', expected_outputs)
 def test_dynamic(dynamic_server, filename, expected_output):
     response = urllib2.urlopen(dynamic_server + filename)
     assert response.read() == expected_output
 
 
-def test_revision_arg(output_pages):
-    assert 'bar' not in output_pages[0]
+def test_revision_arg(revision, output_pages):
+    if revision is None:
+        assert 'bar' in output_pages
+    else:
+        assert 'bar' not in output_pages