-
Notifications
You must be signed in to change notification settings - Fork 93
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rad group switch output #7488
rad group switch output #7488
Conversation
@@ -156,6 +158,7 @@ func Test_Run(t *testing.T) { | |||
ConnectionFactory: &connections.MockFactory{ApplicationsManagementClient: appManagementClient}, | |||
Workspace: workspace, | |||
UCPResourceGroupName: "a", | |||
Output: outputSink, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add an assert below to verify the output?
Here's an example you can borrow from: https://github.com/radius-project/radius/blob/main/pkg/cli/cmd/app/delete/delete_test.go#L280
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you update the test to verify the console output? Other than that change looks great!
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7488 +/- ##
==========================================
- Coverage 63.83% 63.83% -0.01%
==========================================
Files 417 417
Lines 23497 23501 +4
==========================================
+ Hits 14999 15001 +2
- Misses 7228 7229 +1
- Partials 1270 1271 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for your contribution!
@@ -128,6 +131,12 @@ func (r *Runner) Run(ctx context.Context) error { | |||
|
|||
return nil | |||
}) | |||
return err | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: extra line can be removed
You also need a rebase/merge with the main branch. |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
rad cli group switch output to display success message when switching output to a group fixes #7395 Signed-off-by: gpltaylor <gpltaylor@gmail.com>
Signed-off-by: gpltaylor <gpltaylor@gmail.com>
Signed-off-by: gpltaylor <gpltaylor@gmail.com>
Signed-off-by: gpltaylor <gpltaylor@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks @gpltaylor
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
@rynowak hows this looking? |
Th
I'm merging this now. Thanks again. |
# Description rad cli group switch output to display success message when switching output to a group ## Type of change - This pull request fixes a bug in Radius and has an approved issue (issue link required). Fixes: #7395 --------- Signed-off-by: gpltaylor <gpltaylor@gmail.com>
Description
rad cli group switch output to display success message when switching output to a group
Type of change
Fixes: #7395