From 2357b878970038f32360bfc3528afac53464f3cc Mon Sep 17 00:00:00 2001 From: Luis Plazas Date: Wed, 8 Apr 2020 19:17:15 -0700 Subject: [PATCH 1/4] Remove extra parsing --- main.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/main.go b/main.go index dd998822..dbe52c50 100644 --- a/main.go +++ b/main.go @@ -17,7 +17,6 @@ limitations under the License. package main import ( - "flag" "fmt" "os" @@ -28,7 +27,6 @@ import ( const containerDiffEnvPrefix = "CONTAINER_DIFF_ENABLE_PROFILING" func main() { - flag.Parse() if os.Getenv(containerDiffEnvPrefix) == "1" { defer profile.Start(profile.TraceProfile).Stop() } From 53d1c0ee265a29dcb61f521c42d6cf449db751d6 Mon Sep 17 00:00:00 2001 From: Luis Plazas Date: Thu, 9 Apr 2020 14:30:45 -0700 Subject: [PATCH 2/4] Add unit tests for CLI help commands. --- tests/integration_test.go | 124 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 124 insertions(+) diff --git a/tests/integration_test.go b/tests/integration_test.go index acd1ac54..e23dcbdf 100644 --- a/tests/integration_test.go +++ b/tests/integration_test.go @@ -284,3 +284,127 @@ func TestMain(m *testing.M) { closer.Close() os.Exit(m.Run()) } + +func TestConsoleOutput(t *testing.T) { + runner := ContainerDiffRunner{ + t: t, + binaryPath: "../out/container-diff", + } + + tests := []struct { + description string + subCommand string + extraFlag string + expectedOutput []string + producesError bool + }{ + { + description: "analysis --help", + subCommand: "analyze", + extraFlag: "--help", + expectedOutput: []string{ + "Analyzes an image using the specifed analyzers as indicated via --type flag(s).", + "For details on how to specify images, run: container-diff help", + "container-diff analyze image [flags]", + "-c, --cache-dir string cache directory base to create .container-diff (default is $HOME).", + "-j, --json JSON Output defines if the diff should be returned in a human readable format (false) or a JSON (true).", + "-w, --output string output file to write to (default writes to the screen).", + "-t, --type multiValueFlag This flag sets the list of analyzer types to use.", + }, + }, + { + description: "analysis help", + subCommand: "analyze", + extraFlag: "help", + expectedOutput: []string{ + "Analyzes an image using the specifed analyzers as indicated via --type flag(s).", + "For details on how to specify images, run: container-diff help", + "container-diff analyze image [flags]", + "-c, --cache-dir string cache directory base to create .container-diff (default is $HOME).", + "-j, --json JSON Output defines if the diff should be returned in a human readable format (false) or a JSON (true).", + "-w, --output string output file to write to (default writes to the screen).", + "-t, --type multiValueFlag This flag sets the list of analyzer types to use.", + }, + }, + { + description: "container-diff --help", + subCommand: "--help", + extraFlag: "", + expectedOutput: []string{ + "container-diff is a CLI tool for analyzing and comparing container images.", + "Images can be specified from either a local Docker daemon, or from a remote registry.", + "analyze Analyzes an image: container-diff image", + "diff Compare two images: container-diff image1 image2", + "--format string Format to output diff in.", + "--skip-tls-verify-registry multiValueFlag Insecure registry ignoring TLS verify to push and pull. Set it repeatedly for multiple registries.", + "-v, --verbosity string This flag controls the verbosity of container-diff. (default \"warning\")", + }, + }, + { + description: "container-diff help", + subCommand: "help", + extraFlag: "", + expectedOutput: []string{ + "container-diff is a CLI tool for analyzing and comparing container images.", + "Images can be specified from either a local Docker daemon, or from a remote registry.", + "analyze Analyzes an image: container-diff image", + "diff Compare two images: container-diff image1 image2", + "--format string Format to output diff in.", + "--skip-tls-verify-registry multiValueFlag Insecure registry ignoring TLS verify to push and pull. Set it repeatedly for multiple registries.", + "-v, --verbosity string This flag controls the verbosity of container-diff. (default \"warning\")", + }, + }, + { + description: "container-diff diff --help", + subCommand: "diff", + extraFlag: "--help", + expectedOutput: []string{ + "Compares two images using the specifed analyzers as indicated via --type flag(s).", + "For details on how to specify images, run: container-diff help", + "container-diff diff image1 image2 [flags]", + "-c, --cache-dir string cache directory base to create .container-diff (default is $HOME).", + "-j, --json JSON Output defines if the diff should be returned in a human readable format (false) or a JSON (true).", + "-w, --output string output file to write to (default writes to the screen).", + "--skip-tls-verify-registry multiValueFlag Insecure registry ignoring TLS verify to push and pull. Set it repeatedly for multiple registries.", + }, + }, + { + description: "container-diff diff --help", + subCommand: "diff", + extraFlag: "help", + expectedOutput: []string{ + "Error: 'diff' requires two images as arguments: container-diff diff [image1] [image2]", + "container-diff diff image1 image2 [flags]", + "-c, --cache-dir string cache directory base to create .container-diff (default is $HOME).", + "-j, --json JSON Output defines if the diff should be returned in a human readable format (false) or a JSON (true).", + "-w, --output string output file to write to (default writes to the screen).", + "--skip-tls-verify-registry multiValueFlag Insecure registry ignoring TLS verify to push and pull. Set it repeatedly for multiple registries.", + }, + producesError: true, + }, + } + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + t.Parallel() + args := []string{test.subCommand} + if test.extraFlag != "" { + args = append(args, test.extraFlag) + } + actual, stderr, err := runner.Run(args...) + if err != nil { + if !test.producesError { + t.Fatalf("Error running command: %s. Stderr: %s", err, stderr) + } else { + actual = err.Error() + } + } + actual = strings.TrimSpace(actual) + for _, expectedLine := range test.expectedOutput { + if !strings.Contains(actual, expectedLine) { + t.Errorf("Error actual output does not contain expected line. \n\nExpected: %s\n\n Actual: %s\n\n, Stderr: %s", expectedLine, actual, stderr) + } + } + + }) + } +} From e1c67292e66c514a969f4be323d83302e3c4ed11 Mon Sep 17 00:00:00 2001 From: Luis Plazas Date: Thu, 9 Apr 2020 14:45:09 -0700 Subject: [PATCH 3/4] Switch error logic order for better understanding --- tests/integration_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration_test.go b/tests/integration_test.go index e23dcbdf..00e488f2 100644 --- a/tests/integration_test.go +++ b/tests/integration_test.go @@ -392,10 +392,10 @@ func TestConsoleOutput(t *testing.T) { } actual, stderr, err := runner.Run(args...) if err != nil { - if !test.producesError { - t.Fatalf("Error running command: %s. Stderr: %s", err, stderr) - } else { + if test.producesError { actual = err.Error() + } else { + t.Fatalf("Error running command: %s. Stderr: %s", err, stderr) } } actual = strings.TrimSpace(actual) From d99ada0976c04a34b15a7cda0bdb5817064d8e8f Mon Sep 17 00:00:00 2001 From: Luis Plazas Date: Thu, 9 Apr 2020 15:05:28 -0700 Subject: [PATCH 4/4] Change expected console outputs so that they do not depend on console/platform formatting --- tests/integration_test.go | 56 +++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/tests/integration_test.go b/tests/integration_test.go index 00e488f2..75ba0507 100644 --- a/tests/integration_test.go +++ b/tests/integration_test.go @@ -305,11 +305,11 @@ func TestConsoleOutput(t *testing.T) { expectedOutput: []string{ "Analyzes an image using the specifed analyzers as indicated via --type flag(s).", "For details on how to specify images, run: container-diff help", - "container-diff analyze image [flags]", - "-c, --cache-dir string cache directory base to create .container-diff (default is $HOME).", - "-j, --json JSON Output defines if the diff should be returned in a human readable format (false) or a JSON (true).", - "-w, --output string output file to write to (default writes to the screen).", - "-t, --type multiValueFlag This flag sets the list of analyzer types to use.", + "container-diff", + "-c, --cache-dir string", + "-j, --json", + "-w, --output string", + "-t, --type multiValueFlag", }, }, { @@ -319,11 +319,11 @@ func TestConsoleOutput(t *testing.T) { expectedOutput: []string{ "Analyzes an image using the specifed analyzers as indicated via --type flag(s).", "For details on how to specify images, run: container-diff help", - "container-diff analyze image [flags]", - "-c, --cache-dir string cache directory base to create .container-diff (default is $HOME).", - "-j, --json JSON Output defines if the diff should be returned in a human readable format (false) or a JSON (true).", - "-w, --output string output file to write to (default writes to the screen).", - "-t, --type multiValueFlag This flag sets the list of analyzer types to use.", + "container-diff", + "-c, --cache-dir string", + "-j, --json", + "-w, --output string", + "-t, --type multiValueFlag", }, }, { @@ -333,11 +333,11 @@ func TestConsoleOutput(t *testing.T) { expectedOutput: []string{ "container-diff is a CLI tool for analyzing and comparing container images.", "Images can be specified from either a local Docker daemon, or from a remote registry.", - "analyze Analyzes an image: container-diff image", - "diff Compare two images: container-diff image1 image2", - "--format string Format to output diff in.", - "--skip-tls-verify-registry multiValueFlag Insecure registry ignoring TLS verify to push and pull. Set it repeatedly for multiple registries.", - "-v, --verbosity string This flag controls the verbosity of container-diff. (default \"warning\")", + "analyze", + "diff", + "--format string", + "--skip-tls-verify-registry multiValueFlag", + "-v, --verbosity string", }, }, { @@ -347,11 +347,11 @@ func TestConsoleOutput(t *testing.T) { expectedOutput: []string{ "container-diff is a CLI tool for analyzing and comparing container images.", "Images can be specified from either a local Docker daemon, or from a remote registry.", - "analyze Analyzes an image: container-diff image", - "diff Compare two images: container-diff image1 image2", - "--format string Format to output diff in.", - "--skip-tls-verify-registry multiValueFlag Insecure registry ignoring TLS verify to push and pull. Set it repeatedly for multiple registries.", - "-v, --verbosity string This flag controls the verbosity of container-diff. (default \"warning\")", + "analyze", + "diff", + "--format string", + "--skip-tls-verify-registry multiValueFlag", + "-v, --verbosity string", }, }, { @@ -362,10 +362,10 @@ func TestConsoleOutput(t *testing.T) { "Compares two images using the specifed analyzers as indicated via --type flag(s).", "For details on how to specify images, run: container-diff help", "container-diff diff image1 image2 [flags]", - "-c, --cache-dir string cache directory base to create .container-diff (default is $HOME).", - "-j, --json JSON Output defines if the diff should be returned in a human readable format (false) or a JSON (true).", - "-w, --output string output file to write to (default writes to the screen).", - "--skip-tls-verify-registry multiValueFlag Insecure registry ignoring TLS verify to push and pull. Set it repeatedly for multiple registries.", + "-c, --cache-dir string", + "-j, --json", + "-w, --output string", + "--skip-tls-verify-registry multiValueFlag", }, }, { @@ -375,10 +375,10 @@ func TestConsoleOutput(t *testing.T) { expectedOutput: []string{ "Error: 'diff' requires two images as arguments: container-diff diff [image1] [image2]", "container-diff diff image1 image2 [flags]", - "-c, --cache-dir string cache directory base to create .container-diff (default is $HOME).", - "-j, --json JSON Output defines if the diff should be returned in a human readable format (false) or a JSON (true).", - "-w, --output string output file to write to (default writes to the screen).", - "--skip-tls-verify-registry multiValueFlag Insecure registry ignoring TLS verify to push and pull. Set it repeatedly for multiple registries.", + "-c, --cache-dir string", + "-j, --json", + "-w, --output string", + "--skip-tls-verify-registry multiValueFlag", }, producesError: true, },