karatelabs/karate

should match each fail if the array is empty

ptrthomas opened this issue · 10 comments

this has come up a few times and new users run into this a little too often for comfort

I think it is fair that the intent is always to validate more than one item in the array.

example: https://stackoverflow.com/q/60638091/143475

So match each does not perform path validation, only content, if it exists. Which means if the user is uncertain of their https response, they will need to do some schema validation first.

I actually was not aware of this either. But from a best practice perspective, we should start out with the most important assertions first, such as expected data existing, before moving onto validating the content of that data. Layers of assertions can seem painful, but when it comes to debugging, it can be really helpful to break down the response in this way.

Although we do have all-in-one's like contains only so I can see a "spoiled" community wanting the same here.

this is implemented, I am a bit worried if teams extensively using nested schemas will have issues (see picture below). in which case we may need a "lenient" match each only for the short-cut. for example: #[?] foo

image

actual test:

Scenario: complex nested arrays
* def json =
"""
{
  "foo": {
    "bars": [
      { "barOne": "dc", "barTwos": [{ title: 'blah' }] },
      { "barOne": "dc", "barTwos": [{ title: 'blah' }], barThrees: [{ num: 1 }] }
    ]
  }
}
"""
* def barTwo = { title: '#string' }
* def barThree = { num: '#number' }
* def bar = { barOne: '#string', barTwos: '#[] barTwo', barThrees: '##[] barThree' }
* match json.foo.bars == '#[] bar'

I am a bit worried if teams extensively using nested schemas will have issues

@ptrthomas Just providing some feedback: This is indeed problematic for me.

I just had a CI fail because I didn't pin the karate version (on purpose I think we should use latest).
As a workaround I pinned the version to 1.4.1.RC2 for the time being.

13:44:38.275 [main] ERROR com.intuit.karate - test/integration/tests/notification.util.feature:11
And match each response.docs == call read('../schemas/notification.schema.js')
match failed: EACH_EQUALS
  $ | match each failed, empty array / list (LIST:MAP)
  []
  {"_id":"#string",...}

This (and other tests) started failing w/ 1.4.1.RC3.

How do I need to update our tests / schemas to prevent test failures for empty arrays?

@steve1337 in at least this case, since $ = [] maybe you should skip this test or just * match response.docs == []

let me know if you run into more issues, we are open to un-doing this change

@ptrthomas Thanks for the fast answer.

I want to provide and explain my use case / impl a bit more because unfortunately this won't work in this case

I have a lot of tests like this

Feature: Resource CRUD

    Scenario: Create multiple resources and verify notification are created correctly

        And def data = call read('notification.util.feature@getAll')
        Then def docs = data.response.docs
        And match docs == `#[0]`
        And def initialNotificationCount = docs.length

        Given url `${API_URL}/resource`
        And request ({ key: "val" })
        When method POST
        Then status 201

        And def data = call read('notification.util.feature@getAll')
        And def docs = data.response.docs
        And match docs == `#[${initialNotificationCount + 1}]`

        Given url `${API_URL}/resource`
        And request ({ key: "val" })
        When method POST
        Then status 201
        
        And def data = call read('notification.util.feature@getAll')
        And def docs = data.response.docs
        And match docs == `#[${initialNotificationCount + 2}]`

Then my util feature like this notification.util.feature

@ignore @report=false
Feature: Notification UTILS

@getAll
Scenario: Get ALL notifications for <page> (default = 1)
    * def activityPage = karate.get('page', 1)
    Given url `${API_URL}/activity?page=${activityPage}&sortBy=desc(created)`
    When method GET
    Then status 200
    And match response == call read('../schemas/paginatedResponse.schema.js')
    And match each response.docs == call read('../schemas/notification.schema.js')

I re-use this pattern in a bunch of places as well.

How would you suggest I adjust the code to fit the new changes? I'm not really sure how I would do that because I def want to check the schema once the array is not empty.

@steve1337 One thing you could do is set object to be optional ##(object), then define the object variable as null before performing your first assertion when length is zero. Then set it again for your schema validation, and the rest of the assertions should pass.

all: I've checked in what I think is a reasonable solution for backwards compatibility. those who have extensive use of match each or #[] shortcuts AND where you have some of those run against empty arrays, you can just add * configure matchEachEmptyAllowed = true and get the old behavior where needed

image

adding a comment for future consideration. I think the ideal solution would be to support this syntax:

* match each? actual == expected
* match actual == '#[?] expected'

this was assessed to be too complex to implement at this point. there's a TODO comment in the code for MatchOperation:

Match.Type is currently an enum and should ideally be a composite object. so we have an explosion of enum values to represent all permutations of core type, deep, each and now this "optional each" concept

@steve1337 1.4.1.RC4 is available with the change I described two comments above, so you can add * configure matchEachEmptyAllowed = true only where you want the old behavior, would be great to get your feedback before releasing final

1.4.1 released