Skip to content

fix: suppress JSON output for describe affected --upload#2284

Open
osterman wants to merge 4 commits intomainfrom
osterman/upload-quiet-output
Open

fix: suppress JSON output for describe affected --upload#2284
osterman wants to merge 4 commits intomainfrom
osterman/upload-quiet-output

Conversation

@osterman
Copy link
Copy Markdown
Member

@osterman osterman commented Apr 3, 2026

what

  • When --upload is used with atmos describe affected, the full affected stacks JSON is no longer dumped to the console by default
  • Pass --verbose to see the JSON output, or --file to write it to a file
  • The upload success message now includes a count of affected components (e.g. "Uploaded 12 affected component(s) to Atmos Pro")

why

  • The affected stacks JSON payload can be very large and overwhelms the console when the primary intent is just to upload
  • Users who need the output can opt in with --verbose or --file
  • The summary count provides useful feedback without the noise

references

N/A

Summary by CodeRabbit

  • Bug Fixes
    • Suppressed the large "Affected components and stacks" console output during uploads unless verbose mode or file output is requested, for cleaner feedback.
    • Improved the upload success message to include the number of affected components uploaded.

When --upload is used, the full affected stacks JSON is no longer dumped
to the console by default. Use --verbose to see it. The success message
now includes a count of affected components.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added the size/s Small size PR label Apr 3, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 3, 2026

Dependency Review

✅ No vulnerabilities or license issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA bac173d.
Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

@osterman osterman added the patch A minor, backward compatible change label Apr 3, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 3, 2026

📝 Walkthrough

Walkthrough

Conditionally suppresses the large "Affected components and stacks" output when uploading (unless verbose or an output file is requested) and changes the upload success UI message to include the number of affected components. Adds unit tests covering the upload/output control flow.

Changes

Cohort / File(s) Summary
Upload Output Logic
internal/exec/describe_affected.go
Make viewWithScroll(...) conditional: run only when `!args.Upload
Tests
internal/exec/describe_affected_test.go
Add two tests: one verifies output is suppressed during upload when not verbose and no output file, the other verifies output is shown when verbose; both assert upload still proceeds without error.

Sequence Diagram(s)

(omitted)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • aknysh
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and accurately summarizes the main change: suppressing JSON output when using describe affected with the --upload flag.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch osterman/upload-quiet-output

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai bot previously approved these changes Apr 3, 2026
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add two tests proving that:
- Upload path is reached when output is suppressed (Upload=true, Verbose=false)
- Output is shown when verbose is enabled (Upload=true, Verbose=true)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added size/m Medium size PR and removed size/s Small size PR labels Apr 3, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
internal/exec/describe_affected_test.go (1)

2079-2168: Consider adding a test for OutputFile scenario.

The new conditional in uploadableQuery has three branches: !args.Upload, args.Verbose, and args.OutputFile != "". The tests cover the first two but not the third. A test verifying that output is shown when Upload=true, Verbose=false, and OutputFile is set would complete the coverage.

Example test case
// TestUploadShowsOutputWhenFileSet verifies that when --upload and --file are both set,
// the JSON output is written to file (printOrWriteToFile is called).
func TestUploadShowsOutputWhenFileSet(t *testing.T) {
	printCalled := false

	d := describeAffectedExec{
		atmosConfig: &schema.AtmosConfiguration{},
		printOrWriteToFile: func(atmosConfig *schema.AtmosConfiguration, format, file string, data any) error {
			printCalled = true
			return nil
		},
		IsTTYSupportForStdout: func() bool { return false },
		pageCreator:           pager.New(),
	}

	headRef := plumbing.NewHashReference("refs/heads/feature", plumbing.NewHash("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))
	baseRef := plumbing.NewHashReference("refs/heads/main", plumbing.NewHash("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"))

	affected := []schema.Affected{
		{Component: "vpc", Stack: "dev"},
	}

	err := d.uploadableQuery(
		&DescribeAffectedCmdArgs{
			Upload:      true,
			Verbose:     false,
			OutputFile:  "/tmp/affected.json",
			Format:      "json",
			CIEventType: "pull_request",
			CLIConfig:   &schema.AtmosConfiguration{},
		},
		"https://github.com/example/repo.git",
		headRef,
		baseRef,
		affected,
	)

	assert.True(t, printCalled, "printOrWriteToFile should be called when --upload and --file are both set")
	assert.NoError(t, err)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/exec/describe_affected_test.go` around lines 2079 - 2168, The test
suite is missing coverage for the upload branch when OutputFile is provided; add
a new unit test that calls describeAffectedExec.uploadableQuery with
DescribeAffectedCmdArgs{Upload:true, Verbose:false,
OutputFile:"/tmp/affected.json", Format:"json", CIEventType:"pull_request",
CLIConfig:...} using a describeAffectedExec whose printOrWriteToFile stub flips
a boolean, then assert printOrWriteToFile was called and that uploadableQuery
returns no error; reference the same setup as
TestUploadSuppressesOutputButStillUploads/TestUploadShowsOutputWhenVerbose
(headRef, baseRef, affected, IsTTYSupportForStdout, pageCreator) to keep tests
consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@internal/exec/describe_affected_test.go`:
- Around line 2079-2168: The test suite is missing coverage for the upload
branch when OutputFile is provided; add a new unit test that calls
describeAffectedExec.uploadableQuery with DescribeAffectedCmdArgs{Upload:true,
Verbose:false, OutputFile:"/tmp/affected.json", Format:"json",
CIEventType:"pull_request", CLIConfig:...} using a describeAffectedExec whose
printOrWriteToFile stub flips a boolean, then assert printOrWriteToFile was
called and that uploadableQuery returns no error; reference the same setup as
TestUploadSuppressesOutputButStillUploads/TestUploadShowsOutputWhenVerbose
(headRef, baseRef, affected, IsTTYSupportForStdout, pageCreator) to keep tests
consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a8cc9315-6a41-4eab-a632-b60332adc9fe

📥 Commits

Reviewing files that changed from the base of the PR and between 7c0339c and 88fffe3.

📒 Files selected for processing (2)
  • internal/exec/describe_affected.go
  • internal/exec/describe_affected_test.go

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 3, 2026

Codecov Report

❌ Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.22%. Comparing base (b51717f) to head (bac173d).

Files with missing lines Patch % Lines
internal/exec/describe_affected.go 40.00% 2 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (40.00%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2284      +/-   ##
==========================================
+ Coverage   77.19%   77.22%   +0.03%     
==========================================
  Files        1045     1045              
  Lines       98388    98389       +1     
==========================================
+ Hits        75951    75984      +33     
+ Misses      18184    18155      -29     
+ Partials     4253     4250       -3     
Flag Coverage Δ
unittests 77.22% <40.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
internal/exec/describe_affected.go 72.28% <40.00%> (+0.09%) ⬆️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch A minor, backward compatible change size/m Medium size PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant