Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests seem to always result in PASS #8

Closed
Travisivart opened this issue Aug 7, 2023 · 8 comments
Closed

Tests seem to always result in PASS #8

Travisivart opened this issue Aug 7, 2023 · 8 comments

Comments

@Travisivart
Copy link

I have a custom resource (csn-test1) which is already defined. I am trying to verify that it has a provisioningStatus of Succeeded. The test yaml file is defined as:

tests:
 - name: verify-csn
   timeout:
     after: 2m
   wait:
     before: 10s
   kube:
    get: csn/csn-test1
   assert:
     matches:
       status:
         provisioningStatus:
           status: Succeeded

It seems like it doesn't matter what the 'status: Succeeded' field is set to, it will always result in a 'PASS' on the test, instead of the expected FAIL. I tried setting the field to 'status: Succ' which should FAIL but the test case still ends in PASS.

In fact, I was able to drop the entire 'matches' so it doesn't have anything to match and it still resulted in PASS. I would have expected it to fail if it has nothing to match on, or at least some sort of compile error if it has an assertion to make but nothing is actually defined for it.

tests:
 - name: verify-csn
   timeout:
     after: 2m
   wait:
     before: 10s
   kube:
    get: csn/csn-test1
   assert:
     matches:

Maybe something like this:
=== RUN TestNetwork
suite_test.go:18: invalid YAML: expected map field at line 9, column 11
--- FAIL: TestNetwork (0.00s)
FAIL
FAIL test/functional-gdt 0.016s
FAIL

Which you get if you define assert with nothing under it like so:

 tests:
 - name: verify-csn
   timeout:
     after: 2m
   wait:
     before: 10s
   kube:
    get: csn/csn-test1
   assert:
@jaypipes
Copy link
Member

jaypipes commented Aug 7, 2023

@Travisivart What version of gdt-dev/kube do you have in your go.mod?

@Travisivart Travisivart changed the title Tests seem to always result in Tests seem to always result in PASS Aug 7, 2023
@Travisivart
Copy link
Author

github.com/gdt-dev/gdt v1.1.0

@jaypipes
Copy link
Member

jaypipes commented Aug 7, 2023

@Travisivart same for gdt-dev/kube?

@Travisivart
Copy link
Author

Yes, same for both of them.

@jaypipes
Copy link
Member

jaypipes commented Aug 7, 2023

OK, I think I've figured out at least part of the reason this is happening.

If I change the kube repository's testdata/matches.yaml file's "deployment-matches-expected-fields` test from this:

  - name: deployment-matches-expected-fields
    timeout:
      after: 20s
    kube:
      get: deployments/nginx
    assert:
      matches:
        spec:
          replicas: 2
          template:
            metadata:
              labels:
                app: nginx
        status:
          readyReplicas: 2

to this:

  - name: deployment-matches-expected-fields
    timeout:
      after: 20s
    kube:
      get: deployments/nginx
    assert:
      matches:
        spec:
          replicas: 2
          template:
            metadata:
              labels:
                app: nginx
        status:
          eadyReplicas: 2

(Note misspelling of "readyReplicas" as "eadyReplicas")

I get the following results when running go test -v:

=== RUN   TestMatches
=== RUN   TestMatches/matches
=== RUN   TestMatches/matches/create-deployment
=== RUN   TestMatches/matches/deployment-matches-expected-fields
=== NAME  TestMatches/matches
    run.go:97: assertion failed: timeout exceeded (20s)
--- FAIL: TestMatches (20.16s)
    --- FAIL: TestMatches/matches (20.13s)
        --- PASS: TestMatches/matches/create-deployment (0.01s)
        --- PASS: TestMatches/matches/deployment-matches-expected-fields (19.82s)

Note, the overall test scenario (matches.yaml) produces a FAIL (as it should), however the failure is from the timeout exceeded instead of the assertion failure that last occurred before the timeout fired. And, worse, the test unit (deployment-matches-expected-fields) returns PASS where it should return FAIL.

@jaypipes
Copy link
Member

jaypipes commented Aug 7, 2023

Yeah, if I run TestMatches with gdtcontext.WithDebug(), I do see the failed assertions:

=== RUN   TestMatches/matches/deployment-matches-expected-fields
    eval.go:110: deployment-matches-expected-fields (try 1 after 10.057µs) ok: false, terminal: false
    eval.go:119: deployment-matches-expected-fields (try 1 after 10.057µs) failure: assertion failed: match field not equal: $.status.eadyReplicas not present in subject
    eval.go:110: deployment-matches-expected-fields (try 2 after 622.627352ms) ok: false, terminal: false
    eval.go:119: deployment-matches-expected-fields (try 2 after 622.627352ms) failure: assertion failed: match field not equal: $.status.eadyReplicas not present in subject
    eval.go:110: deployment-matches-expected-fields (try 3 after 1.615915986s) ok: false, terminal: false
    eval.go:119: deployment-matches-expected-fields (try 3 after 1.615915986s) failure: assertion failed: match field not equal: $.status.eadyReplicas not present in subject
    eval.go:110: deployment-matches-expected-fields (try 4 after 2.850877159s) ok: false, terminal: false
    eval.go:119: deployment-matches-expected-fields (try 4 after 2.850877159s) failure: assertion failed: match field not equal: $.status.eadyReplicas not present in subject
    eval.go:110: deployment-matches-expected-fields (try 5 after 4.171709861s) ok: false, terminal: false
    eval.go:119: deployment-matches-expected-fields (try 5 after 4.171709861s) failure: assertion failed: match field not equal: $.status.eadyReplicas not present in subject
    eval.go:110: deployment-matches-expected-fields (try 6 after 6.708380991s) ok: false, terminal: false
    eval.go:119: deployment-matches-expected-fields (try 6 after 6.708380991s) failure: assertion failed: match field not equal: $.status.eadyReplicas not present in subject
    eval.go:110: deployment-matches-expected-fields (try 7 after 8.65087596s) ok: false, terminal: false
    eval.go:119: deployment-matches-expected-fields (try 7 after 8.65087596s) failure: assertion failed: match field not equal: $.status.eadyReplicas not present in subject
    eval.go:110: deployment-matches-expected-fields (try 8 after 12.553565532s) ok: false, terminal: false
    eval.go:119: deployment-matches-expected-fields (try 8 after 12.553565532s) failure: assertion failed: match field not equal: $.status.eadyReplicas not present in subject

so this is an issue of the gdt-kube Spec.Eval() not returning the last failed assertion properly and instead the Scenario.Run() method is papering over the failed assertions with a timeout.

jaypipes added a commit that referenced this issue Aug 8, 2023
This patch addresses a couple related problems, all with the evaluation
of testing.T failures. When `testing.T.Run()` is executed, a new
goroutine is spawned with a new `testing.T` pointer. This specific
goroutine's `testing.T` pointer needs to have *its* `testing.T.Error()`
method called in order for that sub-test to be marked failed. We were
erroneously calling `testing.T.Error()` within the `Scenario.Run()`
method instead of inside the `Spec.Eval()` method, which resulted in the
test scenario being marked failed instead of the individual test unit.

We address the exec plugin's `Spec.Eval()` in this patch to call
`testing.T.Error()` on any assertion failure however additional patches
are coming for the http and kube plugins.

Finally, I made a change to the `gdterrors.TimeoutExceeded()` function
to allow for an assertion failure message to be supplied to the error
producer, making it easier for folks to see "this test assertion failed
to succeed before a timeout of (duration)".

Addresses Issue #8

Signed-off-by: Jay Pipes <jaypipes@gmail.com>
jaypipes added a commit to gdt-dev/kube that referenced this issue Aug 8, 2023
Brings in gdt@v1.1.1 and ensures that the test units/specs call
`testing.T.Error()` instead of relying on the `Scenario.Run()` to do
that.

Also adds a custom YAML unmarshaler for the `Expect` struct and adds
better parse-time errors for the `matches` field as requested in Issue
8.

Addresses Issue gdt-dev/gdt#8

Signed-off-by: Jay Pipes <jaypipes@gmail.com>
@jaypipes
Copy link
Member

jaypipes commented Aug 8, 2023

OK @Travisivart I've cut v1.1.1 releases of gdt-dev/gdt and gdt-dev/kube that should address this issue. Would you mind updating your go.mod to v1.1.1 and rerunning your test please?

@jaypipes
Copy link
Member

This was fixed in gdt 1.1.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants