Validate Test section to avoid panic if test section is not populated properly

[andreafalzetti]

Proposed changes

Fixes DEV-348

In this PR we're adding a validation layer for the Test section of the Okteto Manifest. Currently, passing empty section, or empty test throws panics.

Important Note: Curently, if no tests are configured, the Okteto CLI will just deploy the application. But there is no logs or anything that informs the user that "nothing" has been done test-wise. With this change we make sure they are informed if they run test without configuring them. It also exit non-zero, so if this was intended to always exit 0 (for CI reasons for example), I can make it a warning.

BEFORE (Empty test section)

NOW

How to validate

Using the following manifests, and run okteto test

Missing test section

dev:
  hello-world:
    image: okteto/golang:1
    command: bash
    sync:
      - .:/usr/src/app
    volumes:
      - /go
      - /root/.cache
    securityContext:
      capabilities:
        add:
          - SYS_PTRACE
    forward:
      - 2345:2345

Empty test section

test:

dev:
  hello-world:
    image: okteto/golang:1
    command: bash
    sync:
      - .:/usr/src/app
    volumes:
      - /go
      - /root/.cache
    securityContext:
      capabilities:
        add:
          - SYS_PTRACE
    forward:
      - 2345:2345

Empty test

test:
  unit:

dev:
  hello-world:
    image: okteto/golang:1
    command: bash
    sync:
      - .:/usr/src/app
    volumes:
      - /go
      - /root/.cache
    securityContext:
      capabilities:
        add:
          - SYS_PTRACE
    forward:
      - 2345:2345

CLI Quality Reminders 🔧

For both authors and reviewers:

• Scrutinize for potential regressions
• Ensure key automated tests are in place
• Build the CLI and test using the validation steps
• Assess Developer Experience impact (log messages, performances, etc)
• If too broad, consider breaking into smaller PRs
• Adhere to our code style and code review guidelines

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 43.20%. Comparing base (452f2e9) to head (8c79f27).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4309      +/-   ##
==========================================
- Coverage   43.22%   43.20%   -0.02%     
==========================================
  Files         371      371              
  Lines       30027    30040      +13     
==========================================
  Hits        12979    12979              
- Misses      15924    15937      +13     
  Partials     1124     1124              

[ifbyol]

Important Note: Curently, if no tests are configured, the Okteto CLI will just deploy the application. But there is no logs or anything that informs the user that "nothing" has been done test-wise. With this change we make sure they are informed if they run test without configuring them. It also exit non-zero, so if this was intended to always exit 0 (for CI reasons for example), I can make it a warning.

What other tools or languages behaves when no tests are defined? I mean, for example image I execute go test in a directory without tests. Does it fails or just print no tests were run and that's it?

[andreafalzetti]

What other tools or languages behaves when no tests are defined? I mean, for example image I execute go test in a directory without tests. Does it fails or just print no tests were run and that's it?

I you run go test in an empty directory, it fails with: exit 1: go: go.mod file not found in current directory or any parent directory; see 'go help modules'.

Once you init the project (go mod init..) it fails exit 1: no Go files in /var/folders/nt/ggnw9z4d1712sftc8606lj7r0000gn/T/tmp.j2E7UpOEcD

[jLopezbarb]

I think this code could be simplified by removing the bool and returning nil as soon as we detect there is one without the break. WDYT?

Suggested change

	hasAtLeastOne = true
	for _, t := range test {
	if t != nil {
	return nil
	}
	}

[andreafalzetti]

@jLopezbarb I personally find more readable and easier to maintain the current code, because having return nil means we cannot add additional validations after this check. If we want to add one extra check in the future, we need to refactor and add the bool again. Your suggestion uses less memory but in our context, I think the benefits are minimal compared to keeping the code easy to change. WDYT?

[jLopezbarb]

@andreafalzetti I know that is out of scope of this PR but I also found one more panic that maybe we could fix it on this PR.
Steps to reproduce:

1. Using the following okteto.yml:

build:
  backend:
    context: .
test:
  backend:
    image: okteto/golang:1
    context: backend
    commands:
      - name: command1
        command: echo "this is command 1"
      - name: command2
        command: echo "this is command 2"

2. Run okteto test

[andreafalzetti]

@andreafalzetti I know that is out of scope of this PR but I also found one more panic that maybe we could fix it on this PR. Steps to reproduce:

1. Using the following `okteto.yml`:

build:
  backend:
    context: .
test:
  backend:
    image: okteto/golang:1
    context: backend
    commands:
      - name: command1
        command: echo "this is command 1"
      - name: command2
        command: echo "this is command 2"

2. Run `okteto test`

Good catch! As discussed offline, we'll tackle that case in DEV-359