-
Notifications
You must be signed in to change notification settings - Fork 102
Conversation
390ca00
to
a28079f
Compare
23064c0
to
4616813
Compare
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 work. I left some comments for my first pass. I'll come back through and mainly take a look at the MockClient
since I didn't have time for it this morning.
pkg/cli/acorn_test.go
Outdated
client: &testdata.MockClient{}, | ||
}, | ||
wantErr: false, | ||
wantOut: "Acorn: Containerized Application Packaging Framework\n\nUsage:\n acorn [flags]\n acorn [command]\n\nAvailable Commands:\n all List (almost) all objects\n app List or get apps\n build Build an app from a Acornfile file\n check Check if the cluster is ready for Acorn\n container Manage containers\n credential Manage registry credentials\n exec Run a command in a container\n help Help about any command\n image Manage images\n info Info about acorn installation\n install Install and configure acorn in the cluster\n login Add registry credentials\n logout Remove registry credentials\n logs Log all pods from app\n pull Pull an image from a remote registry\n push Push an image to a remote registry\n render Evaluate and display an Acornfile with args\n rm Delete an app, container, secret or volume\n run Run an app from an image or Acornfile\n secret Manage secrets\n start Start an app\n stop Stop an app\n tag Tag an image\n uninstall Uninstall acorn and associated resources\n update Update a deployed app\n volume Manage volumes\n wait Wait an app to be ready then exit with status code 0\n\nFlags:\n -A, --all-namespaces Namespace to work in\n --context string Context to use in the kubeconfig file\n --debug Enable debug logging\n --debug-level int Debug log level (valid 0-9) (default 7)\n -h, --help help for acorn\n --kubeconfig string Location of a kubeconfig file\n --namespace string Namespace to work in (default \"acorn\")\n\nUse \"acorn [command] --help\" for more information about a command.\n", |
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 we swap this to be ``` instead? That would make it a bit more natural to read for the future. If we want to cut down on lines, we could alternatively look at creating a testdata
dir like some of the other tests have. Inside of that we could have a sub-directory called `acorn` and in that we could have a file of expect output for each of these tests.
Doing either of those things should make it less of a hassle when adding new tests that have expected console output since we won't need to add any formatting characters.
commandContext client.CommandContext | ||
}{ | ||
{ | ||
name: "acorn render .", fields: fields{ |
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.
We should test the happy path for this as well.
45a73a3
to
0323d94
Compare
b09b75a
to
8e7cb2c
Compare
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.
just need the conflict resolved
Signed-off-by: Joshua Silverio <joshua@acorn.io>
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 work @jsilverio22
Issue: #868