[5416] test(api): Regression and acceptance tests for mounts file/archive endpoints#5455
Open
Yuvakunaal wants to merge 2 commits into
Open
[5416] test(api): Regression and acceptance tests for mounts file/archive endpoints#5455Yuvakunaal wants to merge 2 commits into
Yuvakunaal wants to merge 2 commits into
Conversation
…/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
|
@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
|
|
📝 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
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.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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