-
Notifications
You must be signed in to change notification settings - Fork 25
refactor: update CobraCommand.Execute to CobraCommand.ExecuteContext in tests #58
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #58 +/- ##
=======================================
Coverage 62.92% 62.92%
=======================================
Files 210 210
Lines 22156 22156
=======================================
Hits 13942 13942
Misses 7130 7130
Partials 1084 1084 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| listPkgMock.On("List").Return(nil) | ||
|
|
||
| err := cmd.Execute() | ||
| err := cmd.ExecuteContext(ctx) |
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.
note: This is pretty much what ever change looks like 😆 .Execute() → .ExecuteContext(ctx).
| // TODO: this test may need a stubbed out parent (root) command to get aliasing working | ||
| /* | ||
| func TestPostRunWorkspaceDeprecationMessage(t *testing.T) { |
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.
note: 🪓 Don't think we need this commented out test. We can add a fresh test when want to tackle that test coverage.
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.
Ocuh! Commented codeblocks are often painful to revisit when the full context is in git histories 👾
Thanks for cleaning this up!
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.
Totally agree! The git histories will keep the chronicle in case we ever want to revisit it. 📜
zimeg
left a comment
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! And an awesome follow up to #57 🤖 ✨
It's comforting to know that context has improved so much with this and prior PRs. I'm also unsettled at what might've been before!
Let's merge this soon once things feel right to you so we can develop with these changes. I look forward to this a lot 🎉
| // TODO: this test may need a stubbed out parent (root) command to get aliasing working | ||
| /* | ||
| func TestPostRunWorkspaceDeprecationMessage(t *testing.T) { |
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.
Ocuh! Commented codeblocks are often painful to revisit when the full context is in git histories 👾
Thanks for cleaning this up!
|
|
||
| func TestAppsListCommand(t *testing.T) { | ||
| // Create mocks | ||
| ctx := slackcontext.MockContext(t.Context()) |
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.
😍 Love to see this more!
Summary
This pull request updates all
CobraCommand.Executereferences to useCobraCommand.ExecuteContext.While minor, it will help prevent future code from using the
.Executesyntax and encourage the use of.ExecuteContext. As we begin to rely on the context values, this will help ensure consistent and reliable tests.Requirements