Issue 5250 - Add copyright update script

update-copyright/tests/test_update_copyright.py

 #!/usr/bin/env python3
 
 import os
 import re
 import datetime
 import subprocess
 import shutil
 import urllib.parse
 
 import pytest
@@ skipping 24 matching lines @@
     """"Returns a path to a temporary repo containing one sample file"""
     temp_repo = tmpdir.mkdir('tmp_dir')
     create_repo(str(temp_repo))
     return temp_repo
 
 
 @pytest.fixture()
 def base_dir(tmpdir):
     """Returns a temporary directory that contains one html page and two
     repositories (one with push access, the other without)"""
-    repo = tmpdir.mkdir('tmp_dir')
-    temp_dir = str(repo)
+    tmp_repo = tmpdir.mkdir('tmp_dir')
+    temp_dir = str(tmp_repo)
     subprocess.check_call(['cp', os.path.join(data_path, 'hg_page.html'),
                            temp_dir])
-    repo_1 = temp_dir + '/repo_1'

[Vasily Kuznetsov, 2017/06/27 16:18:51]

Any reason why you're not using `os.path.join` here? Perhaps that would be more
portable and consistent.

[rosie, 2017/07/03 15:33:44]

Done.

[rosie, 2017/07/03 15:33:44]

repo_1 and repo_2 were only needed as strings, so this was a bit shorter, but
you're right about switching to os.path.join being more portable and consistent.

-    repo_2 = temp_dir + '/repo_2'
-    subprocess.check_call(['mkdir', repo_1])

[Vasily Kuznetsov, 2017/06/27 16:18:51]

You can also use `os.mkdir` for this -- simpler than calling an external
process. Also, can this call be moved into `create_repo` function to avoid
duplication here?

[rosie, 2017/07/03 15:33:43]

Done.

[rosie, 2017/07/03 15:33:44]

Switched it to `os.mkdir`. The `create_repo` function is used in both fixtures,
once in `temp_repo`, and twice in `base_dir`. But the `temp_repo` function
doesn't need both `repo_1` and `repo_2`, so I left that part in the fixture.

[Vasily Kuznetsov, 2017/07/03 19:25:45]

It looks like the same pattern is followed in the other fixture, first a
directory is created and then `create_repo` is called on it. Creating the
directory inside of `create_repo` instead would replace these three lines
(creating "repo_1", "repo_2" and "tmp_repo") with just one line inside of the
function. Or am I missing something?

-    subprocess.check_call(['mkdir', repo_2])
+    repo_1 = os.path.join(temp_dir, 'repo_1')
+    repo_2 = os.path.join(temp_dir, 'repo_2')
+    os.mkdir(repo_1)
+    os.mkdir(repo_2)
     create_repo(repo_1)
     create_repo(repo_2)
 
     # Make repo_2 read-only
-    subprocess.check_call(['touch', os.path.join(repo_2, '.hg/hgrc')])

[Vasily Kuznetsov, 2017/06/27 16:18:51]

Do we actually need to touch `hgrc` before we write into it? Wouldn't the next
line just create the file anyway?

[rosie, 2017/07/03 15:33:43]

Done.

     with open(os.path.join(repo_2, '.hg/hgrc'), 'w') as hgrc:
         hook = '[hooks]\npretxnchangegroup = return True'
         hgrc.write(hook)
-    return repo
+    return temp_dir
 
 
 def test_extract_urls():
     data_url = urllib.parse.urljoin('file:///', data_path)
-    urls = [os.path.join(data_url + '/repo_1/'),

[Vasily Kuznetsov, 2017/06/27 16:18:51]

Seems like we're passing only one argument to `os.path.join` here. Wouldn't it
do nothing in this case?

[rosie, 2017/07/03 15:33:44]

Done.

-            os.path.join(data_url + '/repo_2/')]
+    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):
+    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):
             dates = re.search(r'(\d{4})-(\d{4})', year.group(0))
-            assert dates.group(2) == str(datetime.datetime.now().year)
+            if dates.group(2) == str(datetime.datetime.now().year):
+                updated += 1
+
+        # test that non-eyeo copyright information are left alone
+        assert '2014 example' in text
+    # test for copyright information in both strings and comments
+    assert updated == 2
 
 
 def test_hg_commit(temp_repo, temp_dir):
     directory = str(temp_dir)
-    subprocess.check_call(['hg', 'clone', temp_repo.strpath, directory])

[Vasily Kuznetsov, 2017/06/27 16:18:51]

You could also assign `temp_repo.strpath` to a variable, like you do with
`directory` and reduce the duplication a bit.

[rosie, 2017/07/03 15:33:43]

Done.

+    repo = str(temp_repo)
+    subprocess.check_call(['hg', 'clone', repo, directory])
     open(os.path.join(directory, 'foo'), 'w').close()
     subprocess.check_call(['hg', 'add', '--repository', directory])
-    hg_commit(directory, temp_repo.strpath)
+    hg_commit(directory, repo)
 
     # Make sure both files contain the commmit message from hg log
-    log_1 = subprocess.run(['hg', 'log', '--repository', temp_repo.strpath],
+    log_1 = subprocess.run(['hg', 'log', '--repository', repo],
                            stdout=subprocess.PIPE)
     assert 'Noissue - Updated copyright year' in str(log_1.stdout)
 
 
 def test_all(base_dir):
-    main(urllib.parse.urljoin('file:///',
-         base_dir.join('hg_page.html').strpath),
-         base_dir)
+    main(urllib.parse.urljoin('file:///', os.path.join(
+         base_dir, 'hg_page.html')), None)
 
     # assert hg log for repo_1
     log_1 = subprocess.run(['hg', 'log', '--repository',
-                            base_dir.join('repo_1').strpath],
+                            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