skaffold icon indicating copy to clipboard operation
skaffold copied to clipboard

All custom test commands seem to always be executed on file changes for a given image

Open oversizedhat opened this issue 2 years ago • 4 comments

Expected behavior

When declaring a list of custom test commands in skaffold yml its expected that commands only run if the changes are in files defined in their respective dependencies block.

Actual behavior

When 2 or more commands in a custom block for a image are defined in the skaffold yml it seems that ALL commands are executed regardless if the file changes is defined in the specific commands dependency block or not.

This seems to happen regardless if the command has a dependencies block defined or not. I.e. even if command 1 has a dependencies block containing a different path than command 2 both commands are executed if a change is made in any of the paths.

Information

  • Skaffold version: 1.36.1, 1.35.2
  • Operating system: Ubuntu 18.04.6 LTS
  • Installed via: skaffold.dev
  • Contents of skaffold.yaml:
apiVersion: skaffold/v2beta27
kind: Config
build:
  artifacts:
  - image: custom-test-example
test:
  - image: custom-test-example
    custom:
      - command: ./test.sh
        timeoutSeconds: 60
        dependencies:
          paths:
          -  "*_test.go"
          -  "test.sh"
      - command: echo Hello world!!
        #dependencies:
        #  command: echo [\"main_test.go\"] 
deploy:
  kubectl:
    manifests:
      - k8s-*

Steps to reproduce the behavior

It seems easy to reproduce in the example project.

  1. clone this project [email protected]:GoogleContainerTools/skaffold.git
  2. cd examples/custom-tests
  3. Comment out the dependencies block for the Hello world command
  4. skaffold dev
  5. Make any change in main_test.go and observe how both commands are executed in the console.

Same issue if you comment out the dependencies block for the ./test.sh command. Ie both commands still execute on a change in main_test.go.

oversizedhat avatar Mar 13 '22 10:03 oversizedhat

Thanks you for the issue. We would appreciate any help from the community on this issue.

tejal29 avatar Mar 22 '22 19:03 tejal29

Hi @tejal29, I've been playing around with this and got skaffold to run as described here. Personally, I don't think this is an issue but the intended behavior. Dependencies are "associated" to images, therefore all tests will be run if any of the dependencies change, otherwise the dependency model for custom tests would need to be more complex (mandatory dependencies, default deps, etc...). Also the documentation says:

dependencies | additional test-specific file dependencies; changes to these files will re-run this test.

It's specifically mentioned "re-run this test", not re-run this command.

What do you think?

kadern0 avatar Apr 03 '22 00:04 kadern0

@tejal29 @kadern0 Thanks for looking into this! I would however consider it an issue. If you have a few different scopes of testing to be executed it becomes very time consuming to run everything whenever some unrelated test is changed. Also the setup of being able to configure dependency paths for certain test commands does not make sense if the config is disregarded and all tests are executed anyway. Then the dependencies block should be moved upwards instead of being per command.

But maybe I am looking at this the wrong way? As this is not working I get the feeling that no-one is running unit level tests using skaffold. Is that not the purpose?

The GoogleCLoudPlatform microservices sample app is for example running unit level tests in a very custom fashion instead of relying on the skaffold test phase. https://github.com/GoogleCloudPlatform/microservices-demo/blob/main/.github/workflows/ci-master.yaml

oversizedhat avatar Apr 06 '22 09:04 oversizedhat

Hi @oversizedhat . Thank you for the feedback. I think you're right. Dependencies are owned by command, if a dependency change, only the command owns should be run. The expected behavior you describe makes a lot of sense for TDD. I think this is something we should fix. @tejal29 @aaron-prindle

ericzzzzzzz avatar Jun 06 '22 19:06 ericzzzzzzz