Adding new test command #5118

PriyaModali · 2020-12-07T15:54:26Z

Description
Added a new test command for testing the applications. This command takes pre-built artifact(s) as input from a file and runs the tests on them.

Skaffold currently does not support testing the applications locally and it is required for developers to submit their changes to the upstream repo/branch.

The test command:

Inherits the common flags
Prints the command help
Runs the tests for the given skaffold artifacts

Run skaffold test to test the applications.

Added unit tests and integrations tests for the newly added test command.

Usage

skaffold test [flags] [options]

Examples:
# Build the artifacts and collect the tags into a file
skaffold build --file-output=tags.json

# Test those tags
skaffold test --build-artifacts=tags.json

Options:
-a, --build-artifacts=: File containing build result from a previous 'skaffold build --file-output'

Help

a. Without the help command

skaffold test
You need to:
run [skaffold test] with [--build-artifacts <file-output>] for running tests on artifacts from a given file.

b. With the help command

skaffold test ---help
Test the artifacts

Examples:
  # Build the artifacts and collect the tags into a file
  skaffold build --file-output=tags.json

  # Test those tags
  skaffold test --build-artifacts=tags.json

Options:
  -a, --build-artifacts=: File containing build result from a previous 'skaffold build --file-output'
  -f, --filename='skaffold.yaml': Path or URL to the Skaffold config file

Usage:
  skaffold test [flags] [options]

Use "skaffold options" for a list of global command-line options (applies to all commands).

Sample runs

a. For a skaffold project with no test config

skaffold build --file-output ./file
Generating tags...
 - skaffold-example -> skaffold-example:v1.15.0-149-g78fe28295
Checking cache...
 - skaffold-example: Found. Tagging

skaffold test --build-artifacts ./file

b. For a skaffold project with test config

skaffold build --file-output ./file
Generating tags...
 - skaffold-example -> skaffold-example:v1.15.0-149-g78fe28295-dirty
Checking cache...
 - skaffold-example: Found. Tagging

skaffold test --build-artifacts ./file
Testing images...

====================================================
====== Test file: profile_structure_test.yaml ======
====================================================

============================================
====== Test file: structure_test.yaml ======
============================================
=== RUN: File Existence Test: no go binary
--- PASS
duration: 0s
=== RUN: File Existence Test: no local go binary
--- PASS
duration: 0s

=============================================
================== RESULTS ==================
=============================================
Passes:      2
Failures:    0
Duration:    0s
Total tests: 2

PASS

…nd. GoogleContainerTools#5050) 1. Prints the command help 2. Inherits the common flags

…fold into cmd_test

codecov · 2020-12-09T20:11:53Z

Codecov Report

Merging #5118 (3a598c6) into master (ff0d65c) will increase coverage by 0.00%.
The diff coverage is 56.00%.

@@           Coverage Diff           @@
##           master    #5118   +/-   ##
=======================================
  Coverage   72.32%   72.33%           
=======================================
  Files         382      383    +1     
  Lines       13589    13607   +18     
=======================================
+ Hits         9828     9842   +14     
- Misses       3040     3046    +6     
+ Partials      721      719    -2

Impacted Files	Coverage Δ
cmd/skaffold/app/cmd/flags.go	`87.50% <ø> (ø)`
cmd/skaffold/app/tips/tips.go	`54.54% <0.00%> (-12.13%)`	⬇️
pkg/skaffold/runner/runner.go	`0.00% <ø> (ø)`
cmd/skaffold/app/cmd/util.go	`65.71% <50.00%> (-0.96%)`	⬇️
cmd/skaffold/app/cmd/test.go	`53.84% <53.84%> (ø)`
cmd/skaffold/app/cmd/cmd.go	`66.15% <100.00%> (+0.26%)`	⬆️
pkg/skaffold/runner/build_deploy.go	`69.89% <100.00%> (+1.00%)`	⬆️
pkg/skaffold/util/tar.go	`56.00% <0.00%> (+5.33%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ff0d65c...3a598c6. Read the comment docs.

tejal29

Please add an integration test for skaffold test command similar to TestBuildDeploy

Please update the description to be in sync with the code changes.

Please provide description and sample output on what skaffod test does when

For a skaffold project with no test config
for a skaffold project with test config.

cmd/skaffold/app/tips/tips.go

cmd/skaffold/app/cmd/test.go

cmd/skaffold/app/tips/tips.go

…fold into cmd_test

PriyaModali · 2020-12-14T20:54:51Z

Please add an integration test for skaffold test command similar to TestBuildDeploy

Please update the description to be in sync with the code changes.

Please provide description and sample output on what skaffod test does when

For a skaffold project with no test config

for a skaffold project with test config.

Updated the description to reflect the implementation.

nkubala

mostly nits and docs comments here. logic of this is looking good!

cmd/skaffold/app/cmd/test.go

cmd/skaffold/app/cmd/util.go

cmd/skaffold/app/cmd/util_test.go

nkubala · 2020-12-14T21:45:25Z

docs/content/en/docs/references/cli/_index.md

@@ -73,6 +73,7 @@ End-to-end pipelines:

 Pipeline building blocks for CI/CD:
  build             Build the artifacts
+  test              Test the artifacts


I realize you're just pattern matching against what we already have written here so you don't necessarily need to address in this PR, but I think we should try not to use the term artifact in our docs if we can avoid it. it's not really a concept that has meaning outside of skaffold, so it's fine to use in code (e.g. for function names) but we shouldn't use it when talking about real concepts in docs.

this could be something like "Run tests against images built by Skaffold" or something similar.

Added a tracking issue - #5148

integration/test_test.go

pkg/skaffold/runner/build_deploy.go

…fold into cmd_test

addressed comments

nkubala

one thought I had that I wanted to mention here: do we really need to be passing --build-artifacts to a test command?

to me it feels like this command in the future might be used to run unit tests against source code, which doesn't need any built images to run, in which case we wouldn't need to pass any flags here. but, since our current implementation of tests is exclusively container-structure-tests (which obviously need built images to run), our options are either run the build before the test, or pass them through the --build-artifacts flag.

in the future, I think we should consider whether this should be a hard requirement for running tests vs. a configurable option (e.g. needsBuiltImages boolean in the config), but that's out of scope for this PR.

nkubala

I think this we can merge this now. follow up work is being tracked in #5066

Adding new test command does nothing for now (Add skaffold test comma…

2645f48

…nd. GoogleContainerTools#5050) 1. Prints the command help 2. Inherits the common flags

PriyaModali requested a review from a team as a code owner December 7, 2020 15:54

PriyaModali requested a review from MarlonGamez December 7, 2020 15:54

pull-request-size bot added the size/M label Dec 7, 2020

google-cla bot added the cla: yes label Dec 7, 2020

PriyaModali marked this pull request as draft December 7, 2020 15:54

PriyaModali added 8 commits December 8, 2020 20:06

Update test cmd.

cde1870

Updating boilerplate.

d3d1a48

Add all the supporting plumbing for the Test command to be functional.

ec91f3e

Cleanup run cmd.

ca465fa

Merge branch 'master' of https://github.com/GoogleContainerTools/skaf…

4de2eb2

…fold into cmd_test

Merge branch 'master' of https://github.com/GoogleContainerTools/skaf…

e9ca533

…fold into cmd_test

Merge branch 'master' of https://github.com/GoogleContainerTools/skaf…

3f097ca

…fold into cmd_test

Adding updated CLI reference doc for the newly added test command.

e4f829d

pull-request-size bot added size/L and removed size/M labels Dec 9, 2020

tejal29 previously requested changes Dec 9, 2020

View reviewed changes

cmd/skaffold/app/tips/tips.go Outdated Show resolved Hide resolved

cmd/skaffold/app/cmd/test.go Outdated Show resolved Hide resolved

Updating command tips.

06dcd9f

tejal29 reviewed Dec 9, 2020

View reviewed changes

cmd/skaffold/app/tips/tips.go Outdated Show resolved Hide resolved

PriyaModali mentioned this pull request Dec 9, 2020

Making a util to hold the common cmd functionality #5117

Merged

PriyaModali added 6 commits December 9, 2020 14:26

Addressing a few post-submit comments from PR GoogleContainerTools#5117

97ae29b

Adding unit tests for the newly added test command.

44f312a

Merge branch 'master' of https://github.com/GoogleContainerTools/skaf…

c89ad44

…fold into cmd_test

Updating test flags.

795ad82

Updating CLI reference docs.

6d8ea3a

Adding integration tests for the newly added test command.

78fe282

PriyaModali marked this pull request as ready for review December 14, 2020 18:43

PriyaModali mentioned this pull request Dec 14, 2020

Supporting a new test command in skaffold #5066

Closed

5 tasks

nkubala reviewed Dec 14, 2020

View reviewed changes

Addressing 'nit:'s for the review.

629b280

PriyaModali added the kokoro:force-run forces a kokoro re-run on a PR label Dec 15, 2020

kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Dec 15, 2020

PriyaModali added the kokoro:force-run forces a kokoro re-run on a PR label Dec 15, 2020

Merge branch 'master' of https://github.com/GoogleContainerTools/skaf…

3a598c6

…fold into cmd_test

kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Dec 15, 2020

nkubala reviewed Dec 15, 2020

View reviewed changes

nkubala approved these changes Dec 15, 2020

View reviewed changes

nkubala merged commit b5608f8 into GoogleContainerTools:master Dec 15, 2020

PriyaModali mentioned this pull request Dec 17, 2020

Add test command to devloop #5167

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding new test command #5118

Adding new test command #5118

PriyaModali commented Dec 7, 2020 •

edited

Loading

codecov bot commented Dec 9, 2020 •

edited

Loading

tejal29 left a comment •

edited

Loading

PriyaModali commented Dec 14, 2020

nkubala left a comment

nkubala Dec 14, 2020

PriyaModali Dec 15, 2020

nkubala left a comment

nkubala left a comment

Adding new test command #5118

Adding new test command #5118

Conversation

PriyaModali commented Dec 7, 2020 • edited Loading

codecov bot commented Dec 9, 2020 • edited Loading

Codecov Report

tejal29 left a comment • edited Loading

Choose a reason for hiding this comment

PriyaModali commented Dec 14, 2020

nkubala left a comment

Choose a reason for hiding this comment

nkubala Dec 14, 2020

Choose a reason for hiding this comment

PriyaModali Dec 15, 2020

Choose a reason for hiding this comment

nkubala left a comment

Choose a reason for hiding this comment

nkubala left a comment

Choose a reason for hiding this comment

PriyaModali commented Dec 7, 2020 •

edited

Loading

codecov bot commented Dec 9, 2020 •

edited

Loading

tejal29 left a comment •

edited

Loading