[5416] test(api): Regression and acceptance tests for mounts file/archive endpoints - #5455
Conversation
|
@Yuvakunaal is attempting to deploy a commit to the agenta projects Team on Vercel. A member of the Team first needs to authorize it. |
dosubot
Bot
added
the
size:XS
|
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds acceptance coverage for shallow mount file listings and bulk archive exports, plus a unit regression test ensuring archive streaming handles epoch-millisecond modification times correctly. ChangesMount file endpoint coverage
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a86c4045-cafc-4a1f-8cf4-b2e6e6b5fee0
📒 Files selected for processing (2)
api/oss/tests/pytest/acceptance/mounts/test_mounts_basics.pyapi/oss/tests/pytest/unit/test_mounts_file_ops.py
…/archive endpoints Adds a unit regression test pinning the epoch-ms/seconds mtime bug in stream_mounts_archive, plus acceptance coverage for the shallow (depth=1) listing and the download-all archive endpoint (/mounts/files/export) — the two gaps called out in Agenta-AI#5416. _rollup_recent_entries already had coverage from Agenta-AI#5411, so this PR doesn't touch it. Closes Agenta-AI#5416
Clean up FakeMountStorage._mtimes on delete_keys so a recreated key can't inherit a stale timestamp. Assert the zip member's converted date_time explicitly, verified against the real stream_zip output, so a regression that ignores mtime and falls back to datetime.now() would still fail this test.
4bde0c6 to
50bd927
Compare
|
Hi Mahmoud,
Thank you for reviewing and merging my PR!
I noticed you also mentioned me through @all-contributors. Just wanted to
confirm — does this mean I’m now officially listed/recognized as a
contributor to Agenta?
Really happy to contribute, and looking forward to contributing more!
Best,
Yuva kunaal
…On Thu, 30 Jul, 2026, 15:16 Mahmoud Mabrouk, ***@***.***> wrote:
Merged #5455 <#5455> into
release/v0.106.1.
—
Reply to this email directly, view it on GitHub
<#5455?email_source=notifications&email_token=A6D5IHJVRDWD7VEUU7IFF335HMKOTA5CNFSNUABQM5UWIORPF5TWS5BNNB2WEL2JONZXKZKFOZSW45CON52GSZTJMNQXI2LPNYXTEOBXGEZDMMBXGYYDPJTSMVQXG33OU5WWK3TUNFXW5JLFOZSW45FMMZXW65DFOJPWG3DJMNVQ#event-28712607607>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A6D5IHKAE2IWW2234KEUKVL5HMKOTAVCNFSNUABFKJSXA33TNF2G64TZHM3DGMRYG4YTMMRYHNEXG43VMU5TIOJVGU2TSOJUGYZ2C5QC>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@all-contributors please add @Yuvakunaal for bug |
|
I've put up a pull request to add @Yuvakunaal! 🎉 |
|
@Yuvakunaal yes :) #5568 you will be in the readme as a contributor |
|
Thank you so much.. |
3 participants
Closes #5416
Summary
The drive/mounts refactor (#5400, refined pre-merge by #5411/#5412) shipped with thin
regression coverage on two behaviors: the archive stream's mtime handling, and the new
file-listing/download-all endpoints. #5416 asked for three things; checking the current
suite showed one of them (
_rollup_recent_entriesunit tests) already landed in #5411, sothis PR covers the two that were still open.
Unit regression test for the archive mtime bug.
StoreObject.mtimeis epochmilliseconds, and
stream_mounts_archiveconverts it viadatetime.fromtimestamp(mtime / 1000, ...). Before that division, a realistic ms-scalevalue overflowed
datetime.fromtimestampinside the streaming generator, after theresponse's 200 headers were already sent, so the client got a truncated/broken zip with a
success status. The new test stamps a stored file with a realistic epoch-ms mtime, streams
the archive, and unzips the result to check it's byte-correct.
Acceptance coverage for the two endpoints #5400 shipped untested.
GET /mounts/{id}/files?depth=1(the shallow per-directory listing, pluswith_countsandgit_aware/include_gitignored), andPOST /mounts/files/export(the download-allarchive: prefix folding,
source_pathscoping, missing-mount 404, path-traversalrejection, filename in
Content-Disposition).No production code changes. Both additions are test-only, aside from a small additive
set_mtimehelper on the existingFakeMountStoragetest fake.Testing
Verified locally
/1000division and watched it fail withValueError: year 55840 is out of range,then restored the fix.
pytest oss/tests/pytest/unit/test_mounts_file_ops.py: 51 passed (105 across the fullmounts unit suite).
pytest oss/tests/pytest/acceptance/mounts/: 45 passed, 0 skipped.ruff format+ruff check: clean.Added or updated tests
test_mounts_file_ops.py: newTestArchiveMtimeRegression(1 test) pinning the mtimeconversion bug.
test_mounts_basics.py: newTestMountShallowListing(5 tests) andTestMountArchiveExport(6 tests) covering the two previously-untested endpoints._rollup_recent_entriesalready has 5 unit tests from fix(api): eager archive errors, capped count-only listing, rollup tests #5411(
TestRollupRecentEntries), so this PR doesn't touch it.QA follow-up
N/A — test-only change, no user-visible behavior to QA.
Demo
N/A — not a UI change.
Checklist
Contributor Resources