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

Bad assertion of array of objects #465

Closed
mitasov-ra opened this issue Feb 1, 2022 · 7 comments
Closed

Bad assertion of array of objects #465

mitasov-ra opened this issue Feb 1, 2022 · 7 comments

Comments

@mitasov-ra
Copy link

mitasov-ra commented Feb 1, 2022

Describe the bug

  1. Describe which variant of the API are you using: AssertJ
  2. Create a small reproducible example and paste it the the bug report.

This doesn't work:

assertThatJson("""
    [
      {"value": "1", "title": "Entity", "info": "Entity info"},
      {"value": "2", "title": "Column", "info": "Column info"},
      {"value": "3", "title": "Table", "info": "Table info"},
      {"value": "4", "title": "Schema", "info": "Schema info"}
    ]
""")
    .inPath("$[?(@.value =='1')]")
    .isArray.first()
    .isEqualTo(json("""{"value": "1", "title": "Entity", "info": "Entity info"}"""))
[Different value found in node "$[?(@.value =='1')]" check first element] 
expected: {"value":"1","title":"Entity","info":"Entity info"}
 but was: {"info":"Entity info","title":"Entity","value":"1"}

But this does:

assertThatJson("""
    [
      {"value": "1", "title": "Entity", "info": "Entity info"},
      {"value": "2", "title": "Column", "info": "Column info"},
      {"value": "3", "title": "Table", "info": "Table info"},
      {"value": "4", "title": "Schema", "info": "Schema info"}
    ]
""")
    .inPath("$[?(@.value =='1')]")
    .node("[0]")
    .isEqualTo(json("""{"value": "1", "title": "Entity", "info": "Entity info"}"""))
  1. Describe expected behavior

The above examples represent the same logic, and I expect them to work in the same way. It's a bit disappointing that a more readable one doesn't work

@lukas-krecan
Copy link
Owner

lukas-krecan commented Feb 1, 2022

Hi, thanks for reporting. Will try to fix that. The issue is caused by the fact that by calling isArray() you are moving from JsonUnit to AssertJ. And AssertJ does not know anything about Json comparison. I will take a look if there is a way to to change the behavior. In this case I would recommend to use containsOnly() which works.

@lukas-krecan
Copy link
Owner

Latest findings. first, last and element methods are inherited from ListAssert and they return ObjectAssert which is not customizable. We can extend FactoryBasedNavigableListAssert and make those methods return JsonMapAssert but it's not a backward compatible change.

On the other hand it should break backward compatibility only for the methods that do not work anyway, so it may be possible to do. Have to investigate further.

@lukas-krecan
Copy link
Owner

Bad news. In order to change the assert, we would have to change the ELEMENT type in the FactoryBasedNavigableListAssert from Object to Map and that would break lot of stuff.

So the only way out of it is to mark those methods as deprecated and recommend using Json compatible alternatives.

@lukas-krecan
Copy link
Owner

And we can't even deprecate it since we are using the original ListAssert as return type of isArray.

lukas-krecan added a commit that referenced this issue Feb 3, 2022
lukas-krecan added a commit that referenced this issue Feb 3, 2022
lukas-krecan added a commit that referenced this issue Feb 3, 2022
@lukas-krecan
Copy link
Owner

lukas-krecan commented Feb 5, 2022

Fixed in 2.30.0

@mitasov-ra
Copy link
Author

@lukas-krecan I approve your decision to change return type of isArray to JsonListAssert, now isEqualTo works just fine.

But 2.30.0 version broke a lot of code, related to isArray, without giving the alternatives. For example, JsonObjectAssert does not provide such methods as is<Something> (isObject, isArray, etc.), node, when and so on.

I've come up with issue, that migrating the old code to the new API

assertThatJson("""[{"foo": "bar baz"}]""").isArray
.first()
.extracting("foo")
.asString()
.isEqualTo("bar baz")

seems impossible.

It'll be good to have something like

assertThatJson("""[{"foo": "bar baz"}]""").isArray
.first()
.node("foo")
.isEqualTo(value("bar baz"))

or

assertThatJson("""[{"foo": "bar baz"}]""").isArray
.first()
.isObject
.contains(entry("foo", "bar baz")

But at the moment only isEqualTo is available, making such "check only one field" assertions hard to write and even harder to read.

P.S. btw thanks for your attention, it's really cool that you are so involved in this library

lukas-krecan added a commit that referenced this issue Feb 7, 2022
@lukas-krecan
Copy link
Owner

Hi, you are right. There is actually a simplerm more correct solution. Should be available in 2.31.0.

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