Add timing keyword argument for hooks Measure the duration of the sync operation in the Execute method of the Sync command and pass it to post-sync hooks as a standard keyword argument (`sync_duration_seconds`). Updates based on code review: - Update _API_ARGS in hooks.py to allow sync_duration_seconds for post-sync hooks. - Do not cast sync_duration_seconds to int for better granularity. - Update docs/repo-hooks.md to document sync_duration_seconds. - Add unit test for argument validation in test_hooks.py. Test: Ran run_tests using venv python, all 554 tests passed. Bug: TBD Change-Id: Ie29e002a5d283460d993ad96c224dbf4b6d7985c Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/575021 Tested-by: Arif Kasim <arifkasim@google.com> Commit-Queue: Ram Peri <ramperi@google.com> Reviewed-by: Arif Kasim <arifkasim@google.com> Reviewed-by: Gavin Mak <gavinmak@google.com>
diff --git a/docs/repo-hooks.md b/docs/repo-hooks.md index a56f261..696ad5a 100644 --- a/docs/repo-hooks.md +++ b/docs/repo-hooks.md
@@ -163,13 +163,14 @@ The `post-sync.py` file should be defined like: ```py -def main(repo_topdir=None, **kwargs): +def main(repo_topdir=None, sync_duration_seconds=None, **kwargs): """Main function invoked directly by repo. We must use the name "main" as that is what repo requires. Args: repo_topdir: The absolute path to the top-level directory of the repo workspace. + sync_duration_seconds: The duration of the sync operation in seconds. kwargs: Leave this here for forward-compatibility. """ ```
diff --git a/hooks.py b/hooks.py index d6e1e3b..0637dce 100644 --- a/hooks.py +++ b/hooks.py
@@ -25,7 +25,7 @@ # The API we've documented to hook authors. Keep in sync with repo-hooks.md. _API_ARGS = { "pre-upload": {"project_list", "worktree_list"}, - "post-sync": {"repo_topdir"}, + "post-sync": {"repo_topdir", "sync_duration_seconds"}, }
diff --git a/subcmds/sync.py b/subcmds/sync.py index bfbe193..7e7a2d3 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py
@@ -1986,17 +1986,19 @@ def Execute(self, opt, args): errors = [] + start_time = time.time() try: self._ExecuteHelper(opt, args, errors) + sync_duration_seconds = time.time() - start_time except (RepoExitError, RepoChangedException): raise except (KeyboardInterrupt, Exception) as e: raise RepoUnhandledExceptionError(e, aggregate_errors=errors) # Run post-sync hook only after successful sync - self._RunPostSyncHook(opt) + self._RunPostSyncHook(opt, sync_duration_seconds=sync_duration_seconds) - def _RunPostSyncHook(self, opt): + def _RunPostSyncHook(self, opt, sync_duration_seconds=None): """Run post-sync hook if configured in manifest <repo-hooks>.""" hook = RepoHook.FromSubcmd( hook_type="post-sync", @@ -2004,7 +2006,10 @@ opt=opt, abort_if_user_denies=False, ) - success = hook.Run(repo_topdir=self.client.topdir) + success = hook.Run( + repo_topdir=self.client.topdir, + sync_duration_seconds=sync_duration_seconds, + ) if not success: print("Warning: post-sync hook reported failure.")
diff --git a/tests/test_hooks.py b/tests/test_hooks.py index 9d52d18..14c226c 100644 --- a/tests/test_hooks.py +++ b/tests/test_hooks.py
@@ -14,6 +14,9 @@ """Unittests for the hooks.py module.""" +from io import StringIO +import sys + import pytest import hooks @@ -58,3 +61,47 @@ def test_env_interp(shebang: str, interp: str) -> None: """Lines whose shebang launches through `env`.""" assert hooks.RepoHook._ExtractInterpFromShebang(shebang) == interp + + +def test_post_sync_argument_validation() -> None: + """Test that post-sync hook requires exact API arguments.""" + + class FakeProject: + + def __init__(self): + self.worktree = "/some/path" + self.enabled_repo_hooks = ["post-sync"] + + hook = hooks.RepoHook( + hook_type="post-sync", + hooks_project=FakeProject(), + repo_topdir="/topdir", + manifest_url="https://gerrit", + allow_all_hooks=True, + ) + + old_stderr = sys.stderr + sys.stderr = StringIO() + + try: + # Call with missing arg `sync_duration_seconds` + res = hook.Run(repo_topdir="/topdir") + assert res is False + assert "hook 'post-sync' called incorrectly" in sys.stderr.getvalue() + + # Mock _CheckHook and _ExecuteHook to test success path + hook._CheckHook = lambda: None + + executed_kwargs = {} + + def fake_execute(**kw): + executed_kwargs.update(kw) + + hook._ExecuteHook = fake_execute + + res = hook.Run(repo_topdir="/topdir", sync_duration_seconds=12.345) + assert res is True + assert executed_kwargs.get("sync_duration_seconds") == 12.345 + + finally: + sys.stderr = old_stderr