-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add ability to create a clio.Application that runs test assertions instead of cmd.RunE #37
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
a8a8f84
prototype testing utility
wagoodman 86d286a
WIP: first working pass
willmurphyscode 38b0fda
WIP - moving to testutils package
willmurphyscode b1e2587
expose WrapForTesting
willmurphyscode 6703df4
clear initializers
willmurphyscode 7922e66
remove unused sync dependency
willmurphyscode 96ca556
rename testutils to cliotestutils
willmurphyscode aec76c1
rename NewForTesting to NewApplication
willmurphyscode File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
package cliotestutils | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/google/go-cmp/cmp" | ||
"github.com/spf13/cobra" | ||
"github.com/stretchr/testify/assert" | ||
|
||
"github.com/anchore/clio" | ||
) | ||
|
||
// NewApplication takes a testing.T, a clio setup config, and a slice of assertions, and returns | ||
// a clio application that will, instead of setting up commands with their normal RunE, set up commands | ||
// such that the assertions are called with the testing.T after config state is set up by reading flags, | ||
// env vars, and config files. Useful for testing that expected configuration options are wired up. | ||
// Note that initializers will be cleared from the clio setup config, since the initialization may happen | ||
// more than once and affect global state. For necessary global state, a workaround is to set it in a TestingMain. | ||
func NewApplication(t *testing.T, cfg *clio.SetupConfig, assertions ...AssertionFunc) clio.Application { | ||
cfg.Initializers = nil | ||
wagoodman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
a := clio.New(*cfg) | ||
|
||
var asserter assertionClosure = func(cmd *cobra.Command, args []string, cfgs ...any) { | ||
for _, assertion := range assertions { | ||
assertion(t, cmd, args, cfgs...) | ||
} | ||
} | ||
|
||
return &testApplication{ | ||
a, | ||
asserter, | ||
} | ||
} | ||
|
||
type AssertionFunc func(t *testing.T, cmd *cobra.Command, args []string, cfgs ...any) | ||
|
||
func OptionsEquals(wantOpts any) AssertionFunc { | ||
return func(t *testing.T, cmd *cobra.Command, args []string, cfgs ...any) { | ||
assert.Equal(t, len(cfgs), 1) | ||
if d := cmp.Diff(wantOpts, cfgs[0]); d != "" { | ||
t.Errorf("mismatched options (-want +got):\n%s", d) | ||
} | ||
} | ||
} | ||
|
||
type assertionClosure func(cmd *cobra.Command, args []string, cfgs ...any) | ||
|
||
type testApplication struct { | ||
clio.Application | ||
assertion assertionClosure | ||
} | ||
|
||
func (a *testApplication) SetupCommand(cmd *cobra.Command, cfgs ...any) *cobra.Command { | ||
cmd.RunE = func(cmd *cobra.Command, args []string) error { | ||
a.assertion(cmd, args, cfgs...) | ||
return nil | ||
} | ||
return a.Application.SetupCommand(cmd, cfgs...) | ||
} | ||
|
||
func (a *testApplication) SetupRootCommand(cmd *cobra.Command, cfgs ...any) *cobra.Command { | ||
cmd.RunE = func(cmd *cobra.Command, args []string) error { | ||
a.assertion(cmd, args, cfgs...) | ||
return nil | ||
} | ||
return a.Application.SetupRootCommand(cmd, cfgs...) | ||
} | ||
|
||
/* | ||
// TODO: WISHLIST: | ||
1. Helper to wire up a test fixture as the only config file that will be found | ||
2. Set env vars by passing map[string]string (currently possible by caller in test; a helper here would be nice.) | ||
*/ |
This file contains 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
This file contains 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
Oops, something went wrong.
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.
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.
I like the composition approach here - it might get tough to debug this, since the assertions can happen so far away from the thing they are asserting against, but I think that's a second order concern.
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.
👍