-
-
Notifications
You must be signed in to change notification settings - Fork 15
Auto-fetch periodically #72
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
Conversation
jorio
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! Not too different from how I'd have implemented it myself.
We just need some unit tests! You can use testFetchRemote as a starting point.
Feel free to "cheat" on the delays so the unit tests don't actually take 5 minutes to complete; I do this all the time.
(Don't worry too much about flaky Qt 5 tests in the CI for now)
(Notes for later - I don't mind merging a minimal version first, and ironing out the finer details later)Additional thoughts on this feature in increasing order of difficulty:
These are all outside the scope of this specific PR. I don't mind merging the current PR as it is first (as long as we turn off auto-fetch by default and we have a unit test). |
|
I tried to address the first bullet point here in 3bfd8b8, but I'm not sure how to write a unit test. An LLM came up with this, which seems like vaguely the right idea, but the def testTaskTerminationTerminatesProcess(tempDir, mainWindow):
"""Test that terminating a task also terminates its associated process."""
from gitfourchette import settings
delayCmd = ["python3", getTestDataPath("delay-cmd.py"), "--delay", "3", "--", settings.prefs.gitPath]
mainWindow.onAcceptPrefsDialog({"gitPath": shlex.join(delayCmd)})
wd = unpackRepo(tempDir)
barePath = makeBareCopy(wd, addAsRemote="localfs", preFetch=True)
rw = mainWindow.openRepo(wd)
# Start a fetch task that will take 3 seconds due to the delay command
from gitfourchette.tasks.nettasks import FetchRemotes
FetchRemotes.invoke(rw)
waitUntilTrue(lambda: rw.taskRunner.isBusy())
QTest.qWait(200)
assert rw.taskRunner.currentTask is not None
rw.taskRunner.killCurrentTask()
waitUntilTrue(lambda: not rw.taskRunner.isBusy())
assert rw.taskRunner.currentTask is None |
By default, all tasks run synchronously in unit tests. This facilitates writing test scenarios - no need to explicitly wait for each task to finish. However, the real application runs tasks asynchronously so that the user can still interact with the UI while a task is running. In this case, we want to interrupt a task while it's running, so we need async tasks. You can use the def testOngoingAutoFetchDoesntBlockOtherTasks(tempDir, mainWindow, taskThread):
from gitfourchette import settings
gitCmd = settings.prefs.gitPath
delayGitCmd = shlex.join(["python3", getTestDataPath("delay-cmd.py"), "--", settings.prefs.gitPath])
# Enable auto-fetch and make sure it'll keep RepoTaskRunner busy for a few seconds
mainWindow.onAcceptPrefsDialog({
"autoFetch": True,
"autoFetchMinutes": 1,
"gitPath": delayGitCmd,
})
wd = unpackRepo(tempDir)
barePath = makeBareCopy(wd, addAsRemote="localfs", preFetch=True)
with RepoContext(barePath) as bareRepo:
bareRepo.create_branch_on_head("new-remote-branch")
# Open the repo and wait for it to settle
mainWindow.openRepo(wd)
rw = waitForRepoWidget(mainWindow)
# Manually trigger the auto-fetch timer timeout to simulate the timer firing
rw.lastAutoFetchTime = 0
rw.onAutoFetchTimerTimeout()
# Make sure we're auto-fetching right now
assert isinstance(rw.taskRunner.currentTask, AutoFetchRemotes)
# Don't delay git for the next task
mainWindow.onAcceptPrefsDialog({"gitPath": gitCmd})
assert isinstance(rw.taskRunner.currentTask, AutoFetchRemotes) # just making sure a future version of onAcceptPrefsDialog doesn't kill the task...
# Perform a task - any task! - while auto-fetching is in progress.
# It shouldn't be blocked by an ongoing auto-fetch.
triggerMenuAction(mainWindow.menuBar(), "repo/new local branch")
newBranchDialog = waitForQDialog(rw, "new branch", t=NewBranchDialog)
newBranchDialog.ui.nameEdit.setText("not-blocked-by-auto-fetch")
newBranchDialog.accept()
waitUntilTrue(lambda: not rw.taskRunner.isBusy())
assert "not-blocked-by-auto-fetch" in rw.repo.branches.local
assert "localfs/new-remote-branch" not in rw.repo.branches.remote |
Co-authored-by: Iliyas Jorio <iliyas@jor.io>
Co-authored-by: Iliyas Jorio <iliyas@jor.io>
Co-authored-by: Iliyas Jorio <iliyas@jor.io>
|
Thank you for another PR! |
By default, auto-fetch every 5 minutes. Closes #56, except there's no per-remote setting; it just fetches all remotes.
I don't know if this is quite the right way to do this, comments are welcome :)