Issue 5431 - Fix test for copyright update script

update-copyright/tests/test_update_copyright.py

Index: update-copyright/tests/test_update_copyright.py
===================================================================
--- a/update-copyright/tests/test_update_copyright.py
+++ b/update-copyright/tests/test_update_copyright.py
@@ -9,16 +9,29 @@
 
 import pytest
 
 from update_copyright import extract_urls, text_replace, hg_commit, main
 
 data_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'data')
 
 
+@pytest.fixture(scope='module')
+def generate_sample_file():

[Vasily Kuznetsov, 2017/07/20 15:01:30]

It would be a bit more clear to call the fixture `sample_file` and to make it
return the path. This way we make the dependency on the file more explicit and
remove duplication of the code that constructs this path.

[rosie, 2017/08/03 17:52:45]

Done.

+    # Generate sample_file.py programmatically
+    text = '# {}right (C) 2006-2015 eyeo GmbH\n'.format('Copy')

[Vasily Kuznetsov, 2017/07/20 15:01:30]

If is important to have different years in different instances of the copyright
thingie? If not, perhaps you could first generate the whole copyright
construction and then just reuse it. Something like:

    cr = 'Copy' + 'right (C) ....'
    text = '# {}\nvalue = '{}'\n# {}'.format(cr, cr, cr)

[rosie, 2017/08/03 17:52:45]

The third line uses 'example GmbH' instead of 'eyeo GmbH', to test for false
positives. I could format the first two lines as you suggest, but it would be
the same number of lines of code in the end. The current structure is a bit
easier for me to read, but I don't feel strongly about it either way. If you
think it's cleaner to use the one you suggest, I can change it.

[Vasily Kuznetsov, 2017/08/04 16:54:46]

Acknowledged.

+    text += "value = '{}right (C) 2006-2016 Eyeo GmbH'\n".format('Copy')
+    text += '# {}right (C) 2006-2014 example GmbH'.format('Copy')
+    sample_file = open(os.path.join(data_path, 'sample_file.py'), 'w')

[Vasily Kuznetsov, 2017/07/20 15:01:30]

Would it work to create this file in a temporary directory (`tmpdir`) instead?
That would be a bit cleaner and it would get autocleaned by pytest.

[Vasily Kuznetsov, 2017/07/20 15:01:30]

You could use `with` here instead, then you don't need an explicit `close`:

    with open(...) as sample_file:
        sample_file.write(...)

[rosie, 2017/08/03 17:52:45]

Done.

[rosie, 2017/08/03 17:52:45]

Done.

+    sample_file.write(text)
+    sample_file.close()
+    yield generate_sample_file  # teardown
+    subprocess.call(['rm', os.path.join(data_path, 'sample_file.py')])
+
+
 def create_repo(path):
     subprocess.check_call(['hg', 'init', path])
     with open(os.path.join(path, '.hg', 'hgrc'), 'w+') as hgrc:
         set_user = '[ui]\nusername = Test User <test@example.com>'
         hgrc.write(set_user)
     shutil.copy(os.path.join(data_path, 'sample_file.py'), path)
     subprocess.check_call(['hg', 'commit', '-Am', 'Initial commit',
                            '--repository', path])
@@ -62,17 +75,17 @@
 
 def test_extract_urls():
     data_url = urllib.parse.urljoin('file:///', data_path)
     urls = [data_url + '/repo_1/',
             data_url + '/repo_2/']
     assert urls == extract_urls(os.path.join(data_url, 'hg_page.html'))
 
 
-def test_text_replacement(temp_repo):
+def test_text_replacement(generate_sample_file, temp_repo):
     updated = 0
     filename = temp_repo.join('sample_file.py').strpath
     text_replace(temp_repo.strpath, filename)
     with open(filename) as file:
         text = file.read()
         pattern = re.compile(r'(copyright.*?\d{4})(?:-\d{4})?\s+eyeo gmbh',
                              re.I)
         for year in re.finditer(pattern, text):
@@ -108,8 +121,9 @@
     log_1 = subprocess.run(['hg', 'log', '--repository',
                             os.path.join(base_dir, 'repo_1')],
                            stdout=subprocess.PIPE)
     assert 'Noissue - Updated copyright year' in str(log_1.stdout)
 
     # assert the .patch file for repo_2
     assert'Noissue - Updated copyright year' in open('repo_2.patch').read()
     subprocess.call(['rm', 'repo_2.patch'])  # cleanup
+    # subprocess.call(['rm', os.path.join(data_path, 'sample_file.py')])