Migrate put dry-run to renderOperation and unit-test the helper#354
Merged
Conversation
Route renderPlannedPutResults through the shared renderOperation helper, completing the dry-run migration. All three put dry-run entry points (single file, recursive, stdin) funnel through this one function, so the single edit covers them. The closure keeps put's mixed create-directory and upload line rendering, and renderOperation's warnings argument carries the skipped-symlink warnings. Also add direct unit tests for renderOperation, previously covered only transitively through the migrated commands: text mode writes the closure's output verbatim, and JSON mode renders the operation envelope with results and warnings passthrough while skipping the text closure. Verified byte-for-byte against the pre-refactor binary on a live account for file, recursive (mixed lines), stdin, and symlink-warning cases, with no remote mutation.
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
1 participant
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.
Summary
Sixth and final migration PR in the dry-run generalization series. Routes
renderPlannedPutResultsthrough the sharedrenderOperationhelper (#350), completing the migration — every dry-run command now goes through one envelope path. Also adds direct unit tests forrenderOperationitself.Changes
cmd/put.go—renderPlannedPutResultsusesrenderOperation. All three put dry-run entry points (single file, recursive, stdin) funnel through this one function, so the single edit covers them.cmd/dry_run_test.go— direct unit tests forrenderOperation(previously covered only transitively).Why put is the interesting case
put's dry-run emits mixed line types —
Would create directory …interleaved withWould upload a → b— and carries warnings (skipped symlinks). This is the case that motivated keeping text as a per-command closure rather than a struct. It maps cleanly ontorenderOperation: the closure keeps the mixed-line loop, and the helper'swarningsargument carries the symlink warnings.New helper tests
TestRenderOperationTextUsesClosure— text mode writes the closure's output verbatim.TestRenderOperationJSONRendersEnvelopeAndSkipsClosure— JSON mode renders the full envelope (ok/schema_version/command), results passthrough, and warnings passthrough, while not invoking the text closure. Verified to fail if the warnings argument is dropped.