diff --git a/tests/tools/test_file_sync.py b/tests/tools/test_file_sync.py index 72ef2220cd127..0c387bd388984 100644 --- a/tests/tools/test_file_sync.py +++ b/tests/tools/test_file_sync.py @@ -225,6 +225,42 @@ class TestRateLimiting: mgr.sync() assert upload.call_count == 1 + def test_failed_sync_does_not_suppress_next_retry(self, tmp_files, monkeypatch): + """A failed sync must not advance the rate-limit clock. + + Regression: the failure path used to set ``_last_sync_time`` on + rollback, so the next non-forced ``sync()`` within ``sync_interval`` + hit the rate-limit guard and returned early — silently suppressing the + retry the rollback had just prepared and leaving the remote stale. + """ + from tools.environments import file_sync + + clock = {"t": 1000.0} + monkeypatch.setattr(file_sync.time, "monotonic", lambda: clock["t"]) + + upload = MagicMock(side_effect=RuntimeError("transport down")) + mgr = FileSyncManager( + get_files_fn=_make_get_files(tmp_files), + upload_fn=upload, + delete_fn=MagicMock(), + sync_interval=10.0, + ) + + # First sync fails (forced bypasses the guard); state rolls back. + mgr.sync(force=True) + assert upload.call_count >= 1 + + # Transport recovers; advance the clock by LESS than the interval. + upload.reset_mock() + upload.side_effect = None + clock["t"] = 1002.0 # 2s later, < 10s interval + + # The next non-forced cycle must retry, not be rate-limited away. + mgr.sync() + assert upload.call_count == 3, ( + "a failed sync must not rate-limit the next retry" + ) + class TestEdgeCases: def test_empty_file_list(self): diff --git a/tools/environments/file_sync.py b/tools/environments/file_sync.py index 0c7819712ac4f..181e9643799aa 100644 --- a/tools/environments/file_sync.py +++ b/tools/environments/file_sync.py @@ -232,7 +232,12 @@ class FileSyncManager: except Exception as exc: self._synced_files = prev_files self._pushed_hashes = prev_hashes - self._last_sync_time = time.monotonic() + # Do NOT advance _last_sync_time here: a failed cycle rolls state + # back so the next cycle can retry. Bumping the rate-limit clock on + # failure would make the next non-forced sync() return early (the + # guard above), suppressing that retry for up to _sync_interval and + # leaving the remote with stale files — contradicting this method's + # documented "next cycle retries everything" contract. logger.warning("file_sync: sync failed, rolled back state: %s", exc) # ------------------------------------------------------------------