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

feature: add tests for recursive contains #863

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
179 changes: 179 additions & 0 deletions karate-demo/src/test/java/demo/contains.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
Feature: recursive contains

Background:

* def original =
"""
{
"person": {
"name": "Bob",
"address": {
"line1": "address",
"line2": "county",
"previous_address": {
"line1": "previous_address",
"line2": "previous_county"
}
},
"phone": "555-5555"
}
}
"""

* def missing_level1_obj =
"""
{
"person": {
"name": "Bob",
"phone": "555-5555"
}
}
"""

* def missing_level2_string =
"""
{
"person": {
"name": "Bob",
"address": {
"line1": "address",
"previous_address": {
"line1": "previous_address",
"line2": "previous_county"
}
},
"phone": "555-5555"
}
}
"""

* def missing_level1_string =
"""
{
"person": {
"address": {
"line1": "address",
"line2": "county",
"previous_address": {
"line1": "previous_address",
"line2": "previous_county"
}
},
"phone": "555-5555"
}
}
"""

* def just_internal_object =
"""
{
"name": "Bob",
"address": {
"line1": "address",
"line2": "county",
"previous_address": {
"line1": "previous_address",
"line2": "previous_county"
}
},
"phone": "555-5555"
}
"""

* def too_much_info_level2 =
"""
{
"person": {
"name": "Bob",
"address": {
"line1": "address",
"line2": "county",
"line3": "city",
"previous_address": {
"line1": "previous_address",
"line2": "previous_county"
}
},
"phone": "555-5555"
}
}
"""

* def too_much_info_level3 =
"""
{
"person": {
"name": "Bob",
"address": {
"line1": "address",
"line2": "county",
"previous_address": {
"line1": "previous_address",
"line2": "previous_county",
"line3": "city"
}
},
"phone": "555-5555"
}
}
"""

* def missing_level2_obj =
"""
{
"person": {
"name": "Bob",
"address": {
"line1": "address",
"line2": "county"
},
"phone": "555-5555"
}
}
"""


* def missing_level3_string =
"""
{
"person": {
"name": "Bob",
"address": {
"line1": "address",
"line2": "county",
"previous_address": {
"line1": "previous_address"
}
},
"phone": "555-5555"
}
}
"""


Scenario: should be true for the same object
* match original contains original

Scenario: should be true for missing level 1 object
* match original contains missing_level1_obj

Scenario: should be true for missing level 1 string
* match original contains missing_level1_string

Scenario: should be true for missing level 2 object
* match original contains missing_level2_obj

Scenario: should be true for missing level 2 string
* match original contains missing_level2_string

Scenario: should be true for missing level 3 string
* match original contains missing_level3_string

Scenario: should be false for just the internal data
* match original !contains just_internal_object

Scenario: should be false for too much information at level 2
* match original !contains too_much_info_level2

Scenario: should be false for too much information at level 3
* match original !contains too_much_info_level3
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This scenario seems to be failing, saying that both the actual and expected has the same keys.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is hard to follow, so if there is any way you can replicate the problem with just 2 levels please do so. it is fine if you cut and paste into an issue instead of a PR, I'm unclear right now if this is a test issue or a karate issue

Copy link
Author

@Gokuroro Gokuroro Aug 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope this helps:

Scenario: validation should fail
* def original = { "a": { "b": { "c": { "d":1, "e":2 } } } }
* def compared = { "a": { "b": { "c": { "d":1, "e":2, "f":3 } } } }
* match original contains compared

Scenario: validation should pass
* def original = { "a": { "b": { "c": { "d":1, "e":2 } } } }
* def compared = { "a": { "b": { "c": { "d":1, "e":2, "f":3 } } } }
* match original !contains compared

Both scenarios fail with the following errors:

  • contains.feature:181 - path: $.a.b.c, actual: {d=1, e=2}, expected: {d=1, e=2, f=3}, reason: all key-values did not match, expected has un-matched keys: [f]
  • contains.feature:186 - path: $, actual: {a={b={c={d=1, e=2}}}}, NOT expected: {a={b={c={d=1, e=2, f=3}}}}, reason: actual contains all expected key-values

It seems the contains works fine recursively, but !contains does not.

Do you have any technique you'd recommend for expecting a scenario to fail (like expecting an exception when calling a separate scenario, for example)? Since the implementation was for contains and not !contains, I wanted to test failures for the contains instead of this scenario.

Copy link
Member

@ptrthomas ptrthomas Aug 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expecting a scenario to fail

I'm finding this really hard to understand so I need a better example please.

the only thing that comes to mind is you can try the karate.match() API, and it is limited that there is no method for contains etc, and you have to use the short-cuts such as ^. but in theory you can use that to run a match and manually inspect whether it was a pass or fail

or maybe please read this other extended discussion: https://stackoverflow.com/a/57226061/143475 and see the comments thread also

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I think I understand your first point, those are core JUnit test cases. there are a LOT, here you go: https://github.com/intuit/karate/blob/master/karate-core/src/test/java/com/intuit/karate/ScriptTest.java

but no worries, I can add that myself. but really I am regretting accepting your request for recursive now - I knew this would happen :( do you really need !contains to work >_<

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I really don't need !contains to work. I just didn't know how to test for the negative and thought they would return the same result.
On that note though, I'm not thinking this to deeply so I may be missing some edge cases, but can't the !contains match be interpreted as !(result from contains method)?
Either way, don't worry about it for now, I'll look into the scripttest suite to add those cases there instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I'm just ranting a little, it should be solvable. the match logic is some of the most complicated in Karate, feel free to take a look and see if you can fix this. I'm just not in the mood this week ^_^ it is matchJsonOrObject() in Script.java. have fun :)