Merge pull request #662 from jonovik/is-modified-deleted

Skip commits in history without a model file

scripts/update_mock_repo.py

@@ -27,16 +27,17 @@
 
 import logging
 import os
+import tarfile
 from os.path import join
 from shutil import rmtree
-from subprocess import check_output, call
 from tempfile import mkdtemp
-import tarfile
 
-logging.basicConfig(level=logging.INFO)
+import git
+
 LOGGER = logging.getLogger()
 
-def update_mock_repo():
+
+def update_mock_repo(temp_dir):
     """
     Clone and gzip the memote-mock-repo used for CLI and integration tests.
 
@@ -48,43 +49,34 @@ def update_mock_repo():
     target_file = os.path.abspath(
         join("tests", "data", "memote-mock-repo.tar.gz")
     )
-    temp_dir = mkdtemp(prefix='tmp_mock')
-    previous_wd = os.getcwd()
-    try:
-        LOGGER.info("Cloning repository.")
-        os.chdir(temp_dir)
-        check_output(
-            ['git', 'clone',
-             'https://github.com/ChristianLieven/memote-mock-repo.git']
-        )
-        os.chdir('memote-mock-repo/')
-        LOGGER.info("Setting git to ignore filemode changes.")
-        call(
-            ['git', 'config',
-             'core.fileMode', 'false']
-        )
-        call(
-            ['git', 'config',
-             'user.email', 'memote@opencobra.com']
-        )
-        call(
-            ['git', 'config',
-             'user.name', 'memote-bot']
-        )
-    finally:
-        LOGGER.info("Compressing to tarball.")
-        tar = tarfile.open(target_file, "w:gz")
-        tar.add(
-            join(temp_dir, 'memote-mock-repo/'),
+    repo_dir = join(temp_dir, 'memote-mock-repo')
+    LOGGER.info("Cloning repository.")
+    repo = git.Repo.clone_from(
+        'https://github.com/ChristianLieven/memote-mock-repo.git',
+        repo_dir
+    )
+    LOGGER.info("Setting git to ignore filemode changes.")
+    with repo.config_writer() as writer:
+        writer.set_value("core", "fileMode", "false").release()
+        writer.set_value("user", "name", "memote-bot").release()
+        writer.set_value("user", "email", "bot@memote.io").release()
+    LOGGER.info("Compressing to tarball.")
+    with tarfile.open(target_file, "w:gz") as archive:
+        archive.add(
+            repo_dir,
             arcname="memote-mock-repo"
         )
-        tar.close()
-        LOGGER.info("Success!")
-        LOGGER.info("Removing temporary directory.")
-        rmtree(temp_dir)
-        LOGGER.info("Success! The mock repo has been updated.")
-        os.chdir(previous_wd)
+    LOGGER.info("Success! The mock repo has been updated.")
 
 
 if __name__ == "__main__":
-    update_mock_repo()
+    logging.basicConfig(
+        level=logging.INFO,
+        format="[%(levelname)s] %(message)s"
+    )
+    temp_dir = mkdtemp(prefix='tmp_mock')
+    try:
+        update_mock_repo(temp_dir)
+    finally:
+        LOGGER.info("Removing temporary directory.")
+        rmtree(temp_dir)

src/memote/suite/cli/runner.py

@@ -29,7 +29,6 @@
 from time import sleep
 from tempfile import mkdtemp
 from shutil import copy2, move
-from subprocess import check_call
 
 import click
 import click_log
@@ -212,9 +211,9 @@ def is_verbose(arg):
                 "Committing result and changing back to working branch.")
             manager.store(result, commit=previous_cmt.hexsha)
             repo.git.add(".")
-            check_call(
-                ['git', 'commit',
-                 '-m', "chore: add result for {}".format(previous_cmt.hexsha)]
+            repo.git.commit(
+                "--message",
+                "chore: add result for {}".format(previous_cmt.hexsha)
             )
             if is_branch:
                 previous.checkout()
@@ -422,7 +421,7 @@ def history(model, message, rewrite, solver, location, pytest_args, deployment,
     else:
         move(new_location, os.getcwd())
     repo.git.add(".")
-    check_call(['git', 'commit', '-m', message])
+    repo.git.commit("--message", message)
     LOGGER.info("Success!")
     # Checkout the original branch.
     previous.checkout()
@@ -772,12 +771,8 @@ def online(note, github_repository, github_username):
     te.dump_travis_configuration(config, ".travis.yml")
     LOGGER.info("Add, commit and push changes to '.travis.yml' to GitHub.")
     repo.index.add([".travis.yml"])
-    check_call(
-      ['git', 'commit', '-m', "chore: add encrypted GitHub access token"]
-    )
-    check_call(
-      ['git', 'push', '--set-upstream', 'origin', repo.active_branch.name]
-    )
+    repo.git.commit("--message", "chore: add encrypted GitHub access token")
+    repo.git.push("--set-upstream", "origin", repo.active_branch.name)
 
 
 cli.add_command(report)

src/memote/utils.py

@@ -268,7 +268,7 @@ def flatten(list_of_lists):
 
 def is_modified(path, commit):
     """
-    Test whether a given file was modified in a specific commit.
+    Test whether a given file was present and modified in a specific commit.
 
     Parameters
     ----------
@@ -280,10 +280,19 @@ def is_modified(path, commit):
     Returns
     -------
     bool
-        Whether or not the given path is among the modified files.
+        Whether or not the given path is among the modified files
+        and was not deleted in this commit.
 
     """
-    return path in commit.stats.files
+    try:
+        d = commit.stats.files[path]
+        if (d["insertions"] == 0) and (d["deletions"] == d["lines"]):
+            # File was deleted in commit, so cannot be tested
+            return False
+        else:
+            return True
+    except KeyError:
+        return False
 
 
 def stdout_notifications(notifications):

tests/test_for_suite/test_for_cli/test_for_runner.py

@@ -22,9 +22,6 @@
 import os
 from builtins import str
 from os.path import exists, join, dirname, pardir
-from subprocess import check_output, call
-
-import pytest
 
 
 from memote.suite.cli.runner import cli
@@ -176,29 +173,25 @@ def mock_setup_travis_ci(*args, **kwargs):
 
     # Use the Repository from the mock_repo fixture as the origin to clone from
     path2origin = mock_repo[0]
-    originrepo = mock_repo[1]
+    origin_repo = mock_repo[1]
 
     # We have to allow pushing into the 'origin' repo.
     os.chdir(path2origin)
-    check_output(['git', 'config', 'receive.denyCurrentBranch', 'ignore'])
+    with origin_repo.config_writer() as writer:
+        writer.set_value("receive", "denyCurrentBranch", "ignore").release()
 
     # Create a directory at a temporary path to clone the mock_repo into.
     # Cloning configures the mock_repo as the origin of the "local" repo which
     # allows us to push from one local directory to another.
     path2local = join(path2origin, pardir, 'cloned_repo')
     os.mkdir(path2local)
-    localrepo = originrepo.clone(path2local)
+    local_repo = origin_repo.clone(path2local)
     os.chdir(path2local)
 
     # Setting the config for the local repo.
-    call(
-        ['git', 'config',
-         'user.email', 'memote@opencobra.com']
-    )
-    call(
-        ['git', 'config',
-         'user.name', 'memote-bot']
-    )
+    with local_repo.config_writer() as writer:
+        writer.set_value("user", "name", "memote-bot").release()
+        writer.set_value("user", "email", "bot@memote.io").release()
 
     # Build context_settings
     context_settings = ConfigFileProcessor.read_config()
@@ -212,8 +205,8 @@ def mock_setup_travis_ci(*args, **kwargs):
     assert result.exit_code == 0
 
     # Teardown
-    localrepo.git.reset("--hard", 'HEAD~')
-    localrepo.git.push('--force', 'origin', 'master')
+    local_repo.git.reset("--hard", 'HEAD~')
+    local_repo.git.push('--force', 'origin', 'master')
     os.chdir(previous_wd)
 
 

tests/test_for_utils.py

@@ -15,7 +15,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import os
+
 import pytest
+import git
 
 import memote.utils as utils
 
@@ -116,3 +119,48 @@ def test_show_versions(capsys):
     assert lines[7].startswith("Package Versions")
     assert lines[8].startswith("================")
     assert any(l.startswith("memote") for l in lines[9:])
+
+
+@pytest.fixture(scope="function")
+def mock_repo(tmpdir_factory):
+    """Mock repo with history: add, modify, do nothing, delete file."""
+    path = str(tmpdir_factory.mktemp("memote-mock-repo"))
+    repo = git.Repo.init(path)
+    with repo.config_writer() as writer:
+        writer.set_value("user", "name", "memote-bot").release()
+        writer.set_value("user", "email", "bot@memote.io").release()
+    relname = "test_file.txt"
+    absname = os.path.join(path, relname)
+
+    for message in "Add file.", "Modify file.":
+        with open(absname, "w") as f:
+            f.write(message)
+        repo.index.add([relname])
+        repo.git.commit("--message", message)
+
+    with open(os.path.join(path, "unrelated_file.txt"), "w") as f:
+        f.write("Unrelated file.")
+    repo.index.add(["unrelated_file.txt"])
+    repo.git.commit(
+        "--message",
+        "A commit that does not touch {}".format(relname)
+    )
+
+    repo.index.remove([relname], working_tree=True)
+    repo.git.commit("--message", "Delete file.")
+
+    return relname, repo
+
+
+def test_is_modified_deleted(mock_repo):
+    """Don't report file deletion as modification."""
+    relname, repo = mock_repo
+    # File history (newest first): deleted, unchanged, modified, created
+    # Don't report file deletion as "modification"
+    # for purposes of running memote tests on the file.
+    want = False, False, True, True
+    # Convert to tuple so we can compare want == got,
+    # which pytest will introspect helpfully if the assertion fails.
+    got = tuple(utils.is_modified(relname, commit)
+                for commit in repo.iter_commits())
+    assert want == got