Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add test for repeated "import" (clone and pull) from git #150

Merged
merged 1 commit into from
Nov 23, 2021

Conversation

aplaice
Copy link
Collaborator

@aplaice aplaice commented Nov 20, 2021

Diagnose #138 which occurred on Windows. (A lock for the git pack file is often held by a previous clone/pull and not released, so the next pull fails (PermissionError).)

Without the fix from #139, the test indeed fails (as expected/desired)! (See aplaice#2.)

With the fix from #139, the test indeed succeeds (as expected/desired)! (See aplaice#3.)


I'll leave all these PRs open for several days in case there are comments and if not, carefully merge them in the right order (fixes to tests, tests, GitHub actions?, fixes).


Edit: I added GitHub actions before the new test, to double-check that the new test fails on Windows, in the final configuration, not just with my haphazard set of commits, in the fork.

I'm not too happy with the precise implementation of this test, so I'll leave it for a couple of days, in case I can think of some improvements.

@aplaice aplaice changed the title Add test for succesful repeated "import" (clone and pull) from git Add test for repeated "import" (clone and pull) from git Nov 20, 2021
@aplaice aplaice marked this pull request as draft November 20, 2021 19:31
@aplaice aplaice force-pushed the git_import_test branch 2 times, most recently from 69af6f8 to 1bd455c Compare November 20, 2021 19:37
@aplaice aplaice marked this pull request as ready for review November 20, 2021 19:42
@aplaice aplaice force-pushed the git_import_test branch 2 times, most recently from 11a91a7 to 50e42e8 Compare November 22, 2021 12:17
Diagnose Stvad#138 which occurred on Windows.  (A lock for the git pack
file is often held by a previous clone/pull and not released, so the
next pull fails (`PermissionError`).)

The monkey-patching of `get_repo_local_path` is ugly, but far less
complex than partially mocking ConfigSettings.  Alternatively, we
could have modified `get_repo_local_path` to have an extra, optional
argument (say `full_snapshot_path` that'd override the config call),
but modifying the code to fit the test is also ugly.
@aplaice aplaice merged commit a7bb43a into Stvad:master Nov 23, 2021
@aplaice aplaice deleted the git_import_test branch November 23, 2021 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant