From 90a7221c6487badfb567b20c42c0274276ff1976 Mon Sep 17 00:00:00 2001 From: Jason White Date: Fri, 10 Mar 2017 17:19:32 -0500 Subject: [PATCH 1/4] add utility to detect modified python files in git tree --- setup.py | 1 + shopify_python/git_utils.py | 40 ++++++ tests/shopify_python/test_git_utils.py | 172 +++++++++++++++++++++++++ 3 files changed, 213 insertions(+) create mode 100644 shopify_python/git_utils.py create mode 100644 tests/shopify_python/test_git_utils.py diff --git a/setup.py b/setup.py index f2c3651..1e7cf09 100755 --- a/setup.py +++ b/setup.py @@ -40,6 +40,7 @@ 'pylint==1.6.5', 'six>=1.10.0', 'typing>=3.5.3.0', + 'GitPython==2.1.1', ], extras_require={ 'dev': [ diff --git a/shopify_python/git_utils.py b/shopify_python/git_utils.py new file mode 100644 index 0000000..c605f4c --- /dev/null +++ b/shopify_python/git_utils.py @@ -0,0 +1,40 @@ +import os +import typing # pylint: disable=W0611 +from git import repo +from git.refs import head # pylint: disable=W0611 + + +class GitUtilsException(Exception): + pass + +def _remote_origin_master(git_repo): + # type: (repo.Repo) -> head.Head + remote_master = git_repo.heads.master.tracking_branch() + if not remote_master or not remote_master.is_valid(): + raise GitUtilsException("Unable to locate remote branch origin/master") + return remote_master + +def _modified_in_branch(git_repo, other_ref): + # type: (repo.Repo, head.Head) -> typing.List[str] + commit = git_repo.active_branch.commit + modified = [x for x in commit.diff(other_ref.commit, R=True) if not x.deleted_file] + return [x.b_path for x in modified] + +def _file_is_python(path): + # type: (str) -> bool + if path.endswith('.py'): + return True + else: + with open(path) as might_be_python: + line = might_be_python.readline() + return line.startswith('#!') and 'python' in line + +def changed_python_files_in_tree(root_path): + # type: (str) -> typing.List[str] + + git_repo = repo.Repo(root_path) + remote_master = _remote_origin_master(git_repo) + modified = _modified_in_branch(git_repo, remote_master) + abs_modified = [os.path.join(git_repo.working_dir, x) for x in modified] + return [mod for (mod, abs_mod) in zip(modified, abs_modified) + if os.path.exists(abs_mod) and os.path.isfile(abs_mod) and _file_is_python(abs_mod)] diff --git a/tests/shopify_python/test_git_utils.py b/tests/shopify_python/test_git_utils.py new file mode 100644 index 0000000..df4fce0 --- /dev/null +++ b/tests/shopify_python/test_git_utils.py @@ -0,0 +1,172 @@ +import os +import py # pylint: disable=unused-import +import pytest +from git import repo +from shopify_python import git_utils + +@pytest.fixture +def python_file(main_repo): + # type: (repo.Repo) -> str + file_text = "import os\n" \ + "def foo():\n" \ + " return 4\n" + file_path = os.path.join(main_repo.working_dir, 'program.py') + + with open(file_path, 'w') as writing_file: + writing_file.write(file_text) + return file_path + +@pytest.fixture +def python_script(main_repo): + # type: (repo.Repo) -> str + file_text = "#!/usr/bin/env python3\n" \ + "import os\n" \ + "def bar():\n" \ + " return 6\n" + file_path = os.path.join(main_repo.working_dir, 'program') + + with open(file_path, 'w') as writing_file: + writing_file.write(file_text) + return file_path + +@pytest.fixture +def non_python_file(main_repo): + # type: (repo.Repo) -> str + file_text = "[MASTER]\n" \ + "\n" \ + "# Minimum lines number of a similarity.\n" \ + "min-similarity-lines=4\n" + file_path = os.path.join(main_repo.working_dir, 'non_program') + with open(file_path, 'w') as writing_file: + writing_file.write(file_text) + return file_path + +@pytest.fixture +def remote_repo(tmpdir): + # type: ('py.path.LocalPath') -> repo.Repo + return repo.Repo.init(str(tmpdir.join('remote')), bare=True) + +@pytest.fixture +def main_repo(tmpdir, remote_repo): + # type: ('py.path.LocalPath', repo.Repo) -> repo.Repo + to_dir = str(tmpdir.join('main')) + new_repo = repo.Repo.clone_from(remote_repo.git_dir, to_dir) + + file_name = os.path.join(new_repo.working_dir, 'new_file') + open(str(file_name), 'wb').close() + new_repo.index.add([str(file_name)]) + new_repo.index.commit("initial commit") + new_repo.remote('origin').push('master') + + return new_repo + +def test_detects_changed_python_files(main_repo, python_file, python_script): + # type: (repo.Repo, str, str) -> None + + main_repo.create_head('foo').checkout() + main_repo.index.add([python_file, python_script]) + main_repo.index.commit("adding python files") + + changed_files = git_utils.changed_python_files_in_tree(main_repo.working_dir) + assert sorted(changed_files) == ['program', 'program.py'] + +def test_doesnt_include_changed_nonpython_files(main_repo, python_file, non_python_file): + # type: (repo.Repo, str, str) -> None + + main_repo.create_head('foo').checkout() + main_repo.index.add([python_file, non_python_file]) + main_repo.index.commit("adding mixed files") + + changed_files = git_utils.changed_python_files_in_tree(main_repo.working_dir) + assert changed_files == ['program.py'] + +def test_only_include_modified_locally(main_repo, python_file): + # type: (repo.Repo, str) -> None + + starting_commit = main_repo.commit() + + # Add a file and push it to origin/master + origin = main_repo.remote('origin') + other_file = os.path.join(main_repo.working_dir, 'other_file.py') + open(other_file, 'w').close() + main_repo.index.add([other_file]) + remote_master_commit = main_repo.index.commit("adding other file") + origin.push() + + # Set local master back 1 commit + main_repo.active_branch.commit = 'HEAD~1' + + # Create a new branch from here, add files + main_repo.create_head('foo').checkout() + main_repo.index.add([python_file]) + local_master_commit = main_repo.index.commit("adding python file") + + # current commit should have same parents as remote master (diverged tree) + assert local_master_commit.parents == [starting_commit] + assert remote_master_commit.parents == [starting_commit] + assert local_master_commit != remote_master_commit + + # only the one new file is added + assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == ['program.py'] + +def test_cant_find_remote_origin(main_repo, remote_repo): + # type: (repo.Repo, repo.Repo) -> None + main_repo.create_remote('foo', remote_repo.working_dir) + main_repo.delete_remote('origin') + + with pytest.raises(git_utils.GitUtilsException) as ex: + git_utils.changed_python_files_in_tree(main_repo.working_dir) + assert "Unable to locate remote branch origin/master" in ex.exconly() + +def test_cant_find_origin_master(main_repo, remote_repo): + # type: (repo.Repo, repo.Repo) -> None + + remote_foo = remote_repo.create_head('foo') + remote_repo.head.reference = remote_foo + remote_repo.delete_head('master') + + main_repo.remotes[0].fetch(prune=True) + + with pytest.raises(git_utils.GitUtilsException) as ex: + git_utils.changed_python_files_in_tree(main_repo.working_dir) + assert "Unable to locate remote branch origin/master" in ex.exconly() + +def test_dont_include_deleted_files(main_repo, python_file): + # type: (repo.Repo, str) -> None + + origin = main_repo.remote('origin') + main_repo.index.add([python_file]) + main_repo.index.commit("adding python file") + origin.push('master') + + main_repo.create_head('foo').checkout() + main_repo.index.remove([python_file]) + main_repo.index.commit("removing python file") + + assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == [] + +def test_include_modified_files(main_repo, python_file): + # type: (repo.Repo, str) -> None + + origin = main_repo.remote('origin') + main_repo.index.add([python_file]) + main_repo.index.commit("adding python file") + origin.push() + + main_repo.create_head('foo').checkout() + with open(os.path.join(main_repo.working_dir, python_file), 'a') as appending_file: + appending_file.writelines('# Adding comment at the end') + + main_repo.index.add([python_file]) + main_repo.index.commit("modifying python file") + assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == ['program.py'] + +def test_dont_include_uncommited_or_untracked_files(main_repo, python_file): + # type: (repo.Repo, str) -> None + + assert os.path.exists(os.path.join(main_repo.working_dir, 'program.py')) + assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == [] + main_repo.index.add([python_file]) + assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == [] + main_repo.index.commit("adding python file") + assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == ['program.py'] From 6ef0f2272a93ed06094e7a8ddb85b81695eb902b Mon Sep 17 00:00:00 2001 From: Jason White Date: Mon, 13 Mar 2017 16:16:10 -0400 Subject: [PATCH 2/4] extract filename from fixtures --- tests/shopify_python/test_git_utils.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/shopify_python/test_git_utils.py b/tests/shopify_python/test_git_utils.py index df4fce0..df3567f 100644 --- a/tests/shopify_python/test_git_utils.py +++ b/tests/shopify_python/test_git_utils.py @@ -68,7 +68,10 @@ def test_detects_changed_python_files(main_repo, python_file, python_script): main_repo.index.commit("adding python files") changed_files = git_utils.changed_python_files_in_tree(main_repo.working_dir) - assert sorted(changed_files) == ['program', 'program.py'] + assert sorted(changed_files) == [ + os.path.basename(python_script), + os.path.basename(python_file), + ] def test_doesnt_include_changed_nonpython_files(main_repo, python_file, non_python_file): # type: (repo.Repo, str, str) -> None @@ -78,7 +81,7 @@ def test_doesnt_include_changed_nonpython_files(main_repo, python_file, non_pyth main_repo.index.commit("adding mixed files") changed_files = git_utils.changed_python_files_in_tree(main_repo.working_dir) - assert changed_files == ['program.py'] + assert changed_files == [os.path.basename(python_file)] def test_only_include_modified_locally(main_repo, python_file): # type: (repo.Repo, str) -> None @@ -107,7 +110,7 @@ def test_only_include_modified_locally(main_repo, python_file): assert local_master_commit != remote_master_commit # only the one new file is added - assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == ['program.py'] + assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == [os.path.basename(python_file)] def test_cant_find_remote_origin(main_repo, remote_repo): # type: (repo.Repo, repo.Repo) -> None @@ -159,14 +162,14 @@ def test_include_modified_files(main_repo, python_file): main_repo.index.add([python_file]) main_repo.index.commit("modifying python file") - assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == ['program.py'] + assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == [os.path.basename(python_file)] def test_dont_include_uncommited_or_untracked_files(main_repo, python_file): # type: (repo.Repo, str) -> None - assert os.path.exists(os.path.join(main_repo.working_dir, 'program.py')) + assert os.path.exists(os.path.join(main_repo.working_dir, os.path.basename(python_file))) assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == [] main_repo.index.add([python_file]) assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == [] main_repo.index.commit("adding python file") - assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == ['program.py'] + assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == [os.path.basename(python_file)] From 7417cc6b6148fd31ff1b19590a2a92f146a411a2 Mon Sep 17 00:00:00 2001 From: Jason White Date: Mon, 13 Mar 2017 16:28:37 -0400 Subject: [PATCH 3/4] don't check non .py or binary files --- shopify_python/git_utils.py | 15 ++++++++++----- tests/shopify_python/test_git_utils.py | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/shopify_python/git_utils.py b/shopify_python/git_utils.py index c605f4c..40b3761 100644 --- a/shopify_python/git_utils.py +++ b/shopify_python/git_utils.py @@ -1,7 +1,7 @@ import os -import typing # pylint: disable=W0611 +import typing # pylint: disable=unused-import from git import repo -from git.refs import head # pylint: disable=W0611 +from git.refs import head # pylint: disable=unused-import class GitUtilsException(Exception): @@ -25,9 +25,14 @@ def _file_is_python(path): if path.endswith('.py'): return True else: - with open(path) as might_be_python: - line = might_be_python.readline() - return line.startswith('#!') and 'python' in line + _, extension = os.path.splitext(path) + if not extension: + try: + with open(path) as might_be_python: + line = might_be_python.readline() + return line.startswith('#!') and 'python' in line + except (UnicodeDecodeError): + return False def changed_python_files_in_tree(root_path): # type: (str) -> typing.List[str] diff --git a/tests/shopify_python/test_git_utils.py b/tests/shopify_python/test_git_utils.py index df3567f..a3df0a4 100644 --- a/tests/shopify_python/test_git_utils.py +++ b/tests/shopify_python/test_git_utils.py @@ -173,3 +173,28 @@ def test_dont_include_uncommited_or_untracked_files(main_repo, python_file): assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == [] main_repo.index.commit("adding python file") assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == [os.path.basename(python_file)] + +def test_dont_include_scripts_with_extensions(main_repo): + # type: (repo.Repo) -> None + + file_path = os.path.join(main_repo.working_dir, 'other_file.sh') + with open(file_path, 'w') as writing_file: + writing_file.writelines([ + "#!/usr/bin/env python3", + "import os", + "def bar():", + " return 6" + ]) + main_repo.index.add([file_path]) + main_repo.index.commit("adding script file with .sh extension") + assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == [] + +def test_dont_include_binary_files(main_repo): + # type: (repo.Repo) -> None + + file_path = os.path.join(main_repo.working_dir, 'other_file') + with open(file_path, 'wb') as writing_file: + writing_file.write(bytearray(b'{\x03\xff\x00d')) + main_repo.index.add([file_path]) + main_repo.index.commit("adding binary file") + assert git_utils.changed_python_files_in_tree(main_repo.working_dir) == [] From afd7047991331b99394626a86ae9cf203072ce05 Mon Sep 17 00:00:00 2001 From: Jason White Date: Mon, 13 Mar 2017 16:31:00 -0400 Subject: [PATCH 4/4] don't use backslash line continuations --- shopify_python/git_utils.py | 2 +- tests/shopify_python/test_git_utils.py | 34 +++++++++++++++----------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/shopify_python/git_utils.py b/shopify_python/git_utils.py index 40b3761..506ed6e 100644 --- a/shopify_python/git_utils.py +++ b/shopify_python/git_utils.py @@ -31,7 +31,7 @@ def _file_is_python(path): with open(path) as might_be_python: line = might_be_python.readline() return line.startswith('#!') and 'python' in line - except (UnicodeDecodeError): + except UnicodeDecodeError: return False def changed_python_files_in_tree(root_path): diff --git a/tests/shopify_python/test_git_utils.py b/tests/shopify_python/test_git_utils.py index a3df0a4..68a141c 100644 --- a/tests/shopify_python/test_git_utils.py +++ b/tests/shopify_python/test_git_utils.py @@ -7,38 +7,44 @@ @pytest.fixture def python_file(main_repo): # type: (repo.Repo) -> str - file_text = "import os\n" \ - "def foo():\n" \ - " return 4\n" + file_lines = [ + "import os", + "def foo():", + " return 4" + ] file_path = os.path.join(main_repo.working_dir, 'program.py') with open(file_path, 'w') as writing_file: - writing_file.write(file_text) + writing_file.writelines(file_lines) return file_path @pytest.fixture def python_script(main_repo): # type: (repo.Repo) -> str - file_text = "#!/usr/bin/env python3\n" \ - "import os\n" \ - "def bar():\n" \ - " return 6\n" + file_lines = [ + "#!/usr/bin/env python3", + "import os", + "def bar():", + " return 6" + ] file_path = os.path.join(main_repo.working_dir, 'program') with open(file_path, 'w') as writing_file: - writing_file.write(file_text) + writing_file.writelines(file_lines) return file_path @pytest.fixture def non_python_file(main_repo): # type: (repo.Repo) -> str - file_text = "[MASTER]\n" \ - "\n" \ - "# Minimum lines number of a similarity.\n" \ - "min-similarity-lines=4\n" + file_lines = [ + "[MASTER]", + "", + "# Minimum lines number of a similarity.", + "min-similarity-lines=4" + ] file_path = os.path.join(main_repo.working_dir, 'non_program') with open(file_path, 'w') as writing_file: - writing_file.write(file_text) + writing_file.writelines(file_lines) return file_path @pytest.fixture