fix(git): Fix update_repo when there are untracked files (Take 2)

[jfpedroza]

Second attempt at fixing #395

Alternative to #422

I'm going to see if I can add tests in case there are conflicts.

[tony]

@jfpedroza sounds good

[codecov]

Codecov Report

Merging #425 (ab81dff) into master (dcf97f8) will increase coverage by 0.67%.
The diff coverage is 96.77%.

@@            Coverage Diff             @@
##           master     #425      +/-   ##
==========================================
+ Coverage   62.20%   62.87%   +0.67%     
==========================================
  Files          39       39              
  Lines        2918     2947      +29     
  Branches      698      702       +4     
==========================================
+ Hits         1815     1853      +38     
+ Misses        667      659       -8     
+ Partials      436      435       -1     

Impacted Files	Coverage Δ
src/libvcs/sync/git.py	68.75% <50.00%> (+3.30%)	⬆️
tests/sync/test_git.py	96.72% <100.00%> (+0.51%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[jfpedroza]

@tony I'm having some issues for testing what I want. I can't test a rebase with conflicts because the remote repo is not bare and pushing fails. I'll push my changes.

[tony]

I'm having some issues for testing what I want. I can't test a rebase with conflicts because the remote repo is not bare and pushing fails. I'll push my changes.

I'll take a look

[jfpedroza]

Also, testing manually something expected happens. With conflicts with the working directory rebase fails without starting so you can't try to abort it in the except clause.

[tony]

Also, testing manually something expected happens. With conflicts with the working directory rebase fails without starting so you can't try to abort it in the except clause.

Taking a look

P.S. when you test, which command do you run?

Do you have entr(1) installed?

[tony]

re: #425 (comment)

@jfpedroza Try these to re-run tests on file save

No entr(1)

env PYTEST_ADDOPTS="tests/sync/test_git.py::test_repo_update_stash_cases" poetry run make start

Maybe this will work (with in the back):

poetry run make start PYTEST_ADDOPTS="tests/sync/test_git.py::test_repo_update_stash_cases"

Exit on first failure:

env PYTEST_ADDOPTS="-x tests/sync/test_git.py::test_repo_update_stash_cases" poetry run make start

Verbose, full output:

env PYTEST_ADDOPTS="-x -x -vvv tests/sync/test_git.py::test_repo_update_stash_cases" poetry run make start

If you have entr(1):

make watch_test test='tests/sync/test_git.py::test_repo_update_stash_cases'

Exit on first failure:

make watch_test test='tests/sync/test_git.py::test_repo_update_stash_cases -x'

Verbose, full output:

make watch_test test='tests/sync/test_git.py::test_repo_update_stash_cases -x -s -vvv'

[jfpedroza]

I don't have entr installed. I use either make test to run all tests or Neotest (a Neovim plugin) to run a single test or a test file.

I'll take a look at entr.

[tony]

@jfpedroza I will take a look at Neotest

I'm taking a closer look into your specific error now

[tony]

@jfpedroza Rebase so you can get dcf97f8

[tony]

Next, instead of doing:

 def test_repo_update_stash_cases(
     tmp_path: pathlib.Path,
-    git_remote_repo: pathlib.Path,
+    create_git_remote_repo: CreateProjectCallbackFixtureProtocol,
     mocker: MockerFixture,
     has_untracked_files: bool,
     needs_stash: bool,
     has_remote_changes: bool,
 ) -> None:
+    git_remote_repo = create_git_remote_repo()
+
     options = {
         "url": f"file://{git_remote_repo}",
         "dir": tmp_path / "myrepo",

create_git_remote_repo() accepts init_cmd_args:

• By default, it's ['--bare']
• None and [] is equivalent to plain git init
• You can pass any other flags in list format, e.g.['--bare', '--quiet']

My full diff:

diff --git a/tests/sync/test_git.py b/tests/sync/test_git.py
index b67a2e4..4e86938 100644
--- a/tests/sync/test_git.py
+++ b/tests/sync/test_git.py
@@ -121,7 +121,7 @@ def test_repo_git_obtain_full(
                 "url": f"file://{git_remote_repo}",
                 "dir": tmp_path / "myrepo",
                 "vcs": "git",
-            }
+            },
         ],
         [
             create_project,
@@ -156,24 +156,29 @@ def test_repo_update_handle_cases(
     assert mocker.call(["symbolic-ref", "--short", "HEAD"]) not in mocka.mock_calls


-@pytest.mark.parametrize("has_untracked_files,needs_stash,has_remote_changes", [
-    [True, True, True],
-    [True, True, False],
-    [True, False, True],
-    [True, False, False],
-    [False, True, True],
-    [False, True, False],
-    [False, False, True],
-    [False, False, False]
-])
+@pytest.mark.parametrize(
+    "has_untracked_files,needs_stash,has_remote_changes",
+    [
+        [True, True, True],
+        [True, True, False],
+        [True, False, True],
+        [True, False, False],
+        [False, True, True],
+        [False, True, False],
+        [False, False, True],
+        [False, False, False],
+    ],
+)
 def test_repo_update_stash_cases(
     tmp_path: pathlib.Path,
-    git_remote_repo: pathlib.Path,
+    create_git_remote_repo: CreateProjectCallbackFixtureProtocol,
     mocker: MockerFixture,
     has_untracked_files: bool,
     needs_stash: bool,
     has_remote_changes: bool,
 ) -> None:
+    git_remote_repo = create_git_remote_repo()
+
     options = {
         "url": f"file://{git_remote_repo}",
         "dir": tmp_path / "myrepo",

[tony]

@jfpedroza Are you in a better spot? Or should I try to suggest more things?

[jfpedroza]

I was able to fix tests with that. Only thing missing is that git rebase --abort should not be called.

[tony]

@jfpedroza Taking a look

[tony]

@jfpedroza invited to the slack (if that makes it easier)

[jfpedroza]

@tony Never mind, I switched to running the global vcspull at some point.

It now doesn't crash :)

[tony]

I was able to fix tests with that. Only thing missing is that git rebase --abort should not be called.

git rebase --show-current-patch and check for "No rebase in progress?"?

[tony]

For mypy:

diff --git a/tests/sync/test_git.py b/tests/sync/test_git.py
index bea711c..fe25976 100644
--- a/tests/sync/test_git.py
+++ b/tests/sync/test_git.py
@@ -178,13 +178,12 @@ def test_repo_update_stash_cases(
     has_remote_changes: bool,
 ) -> None:
     git_remote_repo = create_git_remote_repo()
-    options = {
-        "url": f"file://{git_remote_repo}",
-        "dir": tmp_path / "myrepo",
-        "vcs": "git",
-    }

-    git_repo: GitSync = GitSync(**options)
+    git_repo: GitSync = GitSync(
+        url=f"file://{git_remote_repo}",
+        dir=tmp_path / "myrepo",
+        vcs="git",
+    )
     git_repo.obtain()  # clone initial repo

     def make_file(filename: str) -> pathlib.Path:

[jfpedroza]

git rebase --show-current-patch and check for "No rebase in progress?"?

I check the error, if it says "Aborting", there is nothing to do.

I believe this is now ready.

[tony]

@jfpedroza Do you want to add a note to CHANGES?

[jfpedroza]

@tony Done.

[tony]

Live in v0.17.0a1