UX: Be clear when tests are not being run

kkweon commented 4 years ago

Expected behavior

Run the tests
Actual behavior
Did not run the tests (simply ignored without any notice)

Information

❯ git pull --rebase
Already up to date.
Current branch master is up to date.

❯ skaffold version
v1.6.0

❯ container-structure-test version
v1.9.0

❯ pwd
/Users/kkweon/github/skaffold/integration/examples/structure-tests

❯ skaffold dev
Listing files to watch...
 - skaffold-example
Generating tags...
 - skaffold-example -> skaffold-example:v1.6.0-18-geaf5b882b
Checking cache...
 - skaffold-example: Found Locally
Tags used in deployment:
 - skaffold-example -> skaffold-example:2896c1c677c9a43d5c03d2708de16b9e595158421d68a9a58d7c6fcaca778ce6
   local images can't be referenced by digest. They are tagged and referenced by a unique ID instead
Starting deploy...
 - pod/getting-started created
Waiting for deployments to stabilize
Deployments stabilized in 27.103809ms
Watching for changes...
[getting-started] Hello world!
[getting-started] Hello world!
[getting-started] Hello world!
^CCleaning up...
 - pod "getting-started" deleted

❯ cat skaffold.yaml
apiVersion: skaffold/v2beta1
kind: Config
build:
  artifacts:
  - image: skaffold-example
test:
  - image: skaffold-example
    structureTests:
      - ./test/*
deploy:
  kubectl:
    manifests:
      - k8s-*
profiles:
  - name: test
    test:
      - image: skaffold-example
        structureTests:
          - ./test/profile_structure_test.yaml

❯ cat test/structure_test.yaml
schemaVersion: 2.0.0

fileExistenceTests:
  - name: 'no local go binary'
    path: /usr/local/bin/go'
    shouldExist: false

Skaffold version: v.1.6.0

Operating system

❯ neofetch
                'c.          kkweon@kkweon-macbookpro2.roam.corp.google.com
             ,xNMM.          ----------------------------------------------
           .OMMMMo           OS: macOS Catalina 10.15.4 19E266 x86_64
           OMMM0,            Host: MacBookPro15,1
 .;loddo:' loolloddol;.      Kernel: 19.4.0
cKMMMMMMMMMMNWMMMMMMMMMM0:    Uptime: 1 day, 1 hour, 30 mins
.KMMMMMMMMMMMMMMMMMMMMMMMWd.    Packages: 124 (brew)
XMMMMMMMMMMMMMMMMMMMMMMMX.      Shell: zsh 5.8
;MMMMMMMMMMMMMMMMMMMMMMMM:       Resolution: 2560x1440
:MMMMMMMMMMMMMMMMMMMMMMMM:       DE: Aqua
.MMMMMMMMMMMMMMMMMMMMMMMMX.      WM: Spectacle
kMMMMMMMMMMMMMMMMMMMMMMMMWd.    CPU: Intel i9-8950HK (12) @ 2.90GHz
.XMMMMMMMMMMMMMMMMMMMMMMMMMMk   GPU: Radeon Pro 560X, Intel UHD Graphics 630
.XMMMMMMMMMMMMMMMMMMMMMMMMK.   Memory: 18405MiB / 32768MiB
kMMMMMMMMMMMMMMMMMMMMMMd
 ;KMMMMMMMWXXWMMMMMMMk.
   .cooc,.    .,coo:.

Contents of skaffold.yaml:

apiVersion: skaffold/v2beta1
kind: Config
build:
  artifacts:
  - image: skaffold-example
test:
  - image: skaffold-example
    structureTests:
      - ./test/*
deploy:
  kubectl:
    manifests:
      - k8s-*
profiles:
  - name: test
    test:
      - image: skaffold-example
        structureTests:
          - ./test/profile_structure_test.yaml

Steps to reproduce the behavior

❯ git clone  https://github.com/GoogleContainerTools/skaffold --depth 1
Cloning into 'skaffold'...
remote: Enumerating objects: 7134, done.
remote: Counting objects: 100% (7134/7134), done.
remote: Compressing objects: 100% (5502/5502), done.
remote: Total 7134 (delta 1770), reused 3973 (delta 1146), pack-reused 0
Receiving objects: 100% (7134/7134), 17.80 MiB | 1.18 MiB/s, done.
Resolving deltas: 100% (1770/1770), done.
Updating files: 100% (6605/6605), done.

❯ cd skaffold/integration/examples/structure-tests
❯ ll
total 40
drwxr-xr-x   8 kkweon  primarygroup   256B Mar 26 10:58 .
drwxr-xr-x  29 kkweon  primarygroup   928B Mar 26 10:58 ..
-rw-r--r--   1 kkweon  primarygroup   144B Mar 26 10:58 Dockerfile
-rw-r--r--   1 kkweon  primarygroup   678B Mar 26 10:58 README.md
-rw-r--r--   1 kkweon  primarygroup   133B Mar 26 10:58 k8s-pod.yaml
-rw-r--r--   1 kkweon  primarygroup   128B Mar 26 10:58 main.go
-rw-r--r--   1 kkweon  primarygroup   347B Mar 26 10:58 skaffold.yaml
drwxr-xr-x   4 kkweon  primarygroup   128B Mar 26 10:58 test

❯ skaffold dev
Listing files to watch...
 - skaffold-example
Generating tags...
 - skaffold-example -> skaffold-example:eaf5b88
Checking cache...
 - skaffold-example: Found. Tagging
Tags used in deployment:
 - skaffold-example -> skaffold-example:2896c1c677c9a43d5c03d2708de16b9e595158421d68a9a58d7c6fcaca778ce6
   local images can't be referenced by digest. They are tagged and referenced by a unique ID instead
Starting deploy...
 - pod/getting-started created
Waiting for deployments to stabilize
Deployments stabilized in 24.169129ms
Watching for changes...
[getting-started] Hello world!
[getting-started] Hello world!
[getting-started] Hello world!
^CCleaning up...
 - pod "getting-started" deleted
^C

tstromberg commented 4 years ago

This does sound suspicious.

dgageot commented 4 years ago

Hi @kkweon I'm going to close that ticket because it works as intended. The reason why it's not running tests is because the image was already in the cache. Which means it was built and passed the tests the previous time you ran Skaffold. If you want to deacticate caching, you can run skaffold with --cache-artifacts=false

tstromberg commented 4 years ago

Re-opening because we should make this clear in the UX

dgageot commented 4 years ago

@tstromberg what do you have in mind?

kkweon commented 4 years ago

As a user I expect the following two:

message in the log that looks like (but put it nicer way)

Checking cache...
 - skaffold-example: Found. Tagging (Skipping the tests since nothing has changed. Run with --cache-artifacts=false to invalidate the cache)

A doc should address this more explicitly.

https://skaffold.dev/docs/pipeline-stages/testers/

briandealwis commented 4 years ago

TBH I'd prefer the tests to be run unless we run with --skip-tests. The cached image may be from having run with --skip-tests.

briandealwis commented 4 years ago

Further point: our Jib builder does run the in-build tests unless run with --skip-tests.

tejal29 commented 4 years ago

So, in this case. we cant to run test even if artifact is found in the cache.

Split BuildAndTest() into individual Build() and Test().

This will make sure, Build() does nothing when an artifact is cached. Test() should run tests on cached artifact.

From the code, look like we are running tests for cached artifacts

Verify, if this code path is exercised correctly in skaffold build, skaffold dev

gsquared94 commented 4 years ago

This will work: build_deploy.go

// line 64 onwards 
bRes, err := r.cache.Build(ctx, out, tags, artifacts, func(ctx context.Context, out io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact) ([]build.Artifact, error) {
        if len(artifacts) == 0 {
            return nil, nil
        }

        r.hasBuilt = true

        bRes, err := r.builder.Build(ctx, out, tags, artifacts)
        if err != nil {
            return nil, err
        }

-       if !r.runCtx.SkipTests() {
-           if err = r.tester.Test(ctx, out, bRes); err != nil {
-               return nil, err
-           }
-       }
-
        return bRes, nil
    })
    if err != nil {
        return nil, err
    }
+   if !r.runCtx.SkipTests() {
+       if err = r.tester.Test(ctx, out, bRes); err != nil {
+           return nil, err
+       }
+   }

GoogleContainerTools / skaffold