Skip to content

bisync: speed up integration tests on non-local remotes - #10016

Open
nielash wants to merge 2 commits into
rclone:masterfrom
nielash:bisync-test-modtime-probe-once
Open

nielash wants to merge 2 commits into
rclone:masterfrom
nielash:bisync-test-modtime-probe-once

Conversation

@nielash

@nielash nielash commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

What does this change do?

bisync: speed up integration tests on non-local remotes

Before this change, the bisync integration tests checked whether each remote could set modtimes on every bisync step (checkPreReqs, called from runBisync), not once per remote. On a remote that isn't IsLocal, each check sleeps for 1s twice (the Google Cloud Storage per-object rate limit workaround from be73a10), for each side, on top of the check's own API calls. That added 2-4s to each of the ~110 bisync steps in each test.

After this change, the result of the check is remembered for each remote, so it runs, and sleeps, once per remote per test run. The sleeps themselves are unchanged.

bisync: fix Dropbox rate limit notices failing the integration tests

The tests already drop Dropbox's "Too many requests or write operations" notice from the logs, but only in the format it had when the tests were written. Since 110bf46 (v1.65), the notice begins with Error , so the filter stopped matching. Without the sleeps above, the tests run faster and are more likely to be rate limited. This change widens the pattern to match either format.

Benchmarks

Last night's integration test logs, next to runs with this PR:

Remote Integration test server (master) With this PR
TestSFTPRclone: 1010s (log) 66s
TestDrive: Try 1 used most of the 1h timeout, and the 18 TestBisyncRemoteRemote cases it didn't reach ("not enough time to start test") needed a try 2, ~87 min in total (try 1, try 2) 37 min, one try, all passed
TestDropbox: Same pattern, 22 cases retried, ~95 min in total (try 1, try 2) 54 min, one try
:memory: 864s (log) 7s

The "with this PR" runs were on my machine, so only the SFTP row is a like-for-like comparison: master took 1007s there, against 66s with this PR. For Drive and Dropbox, some of the difference is network and account, not this change.

The Dropbox run had four failures, none caused by this change:

  • One was the rate limit notice that the second commit now filters.
  • The other three were Dropbox server errors (HTTP 500 and "unexpected error occurred"), all within 13 minutes of each other.

Those four cases passed on a rerun, both with this PR and on master.

Testing

  • go test ./cmd/bisync -race (local): passes
  • -remote :memory:: 6.8s
  • -remote TestCrypt:: 24.7s
  • TestSFTPRclone:, TestDrive:, TestDropbox:: as above

Linked issue

No issue -- I found this while measuring bisync test coverage against more remotes.

Checklist

  • This change is trivial OR it has been discussed and agreed in the linked issue.
  • I have read the contribution guidelines.
  • (If I used AI tools to help write this code) I have read and understood the AI-assisted contributions guidance, and I have tested and take ownership of this change myself.
  • I have added tests for all changes in this PR if appropriate.
  • I have added documentation for the changes if appropriate.
  • All commit messages are in house style.
  • (Backend changes only) test_all passes for this backend and if submitting a new backend can provide a test account for the integration tester - see CONTRIBUTING.md.
  • This Pull Request is ready for review.

Before this change, the integration tests checked whether each remote could
set modtimes on every `bisync` step (`checkPreReqs`, called from
`runBisync`), not once per remote. On a remote that isn't `IsLocal`, each
check sleeps for 1s twice (the Google Cloud Storage per-object rate limit
workaround from be73a10), for each side. That added 2-4s to each of the
~110 `bisync` steps in each test, which on a fast remote was most of the
run: on the integration test server, the `:memory:` tests took about 14
minutes.

After this change, the result of the check is remembered for each remote,
so it runs, and sleeps, once per remote per test run. The `:memory:` tests
now take about 7s locally.
Before this change, the integration tests dropped Dropbox's "Too many
requests or write operations" notice from the logs only in the format it
had when the tests were written, which began with the error code
(`too_many_requests/...`). Since 110bf46 (v1.65), the notice begins with
`Error `, so it was left in the log and caused a miscompare whenever
Dropbox rate limited a run.

After this change, the notice is dropped in either format.
@nielash nielash added this to the v1.76 milestone Sep 30, 2026
@nielash
nielash marked this pull request as ready for review September 30, 2026 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

1 participant