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

updateMessage on JsonSelectorMatcher yields unexpected results #684

Closed
stilianouly opened this issue Jul 4, 2018 · 4 comments
Closed

updateMessage on JsonSelectorMatcher yields unexpected results #684

stilianouly opened this issue Jul 4, 2018 · 4 comments

Comments

@stilianouly
Copy link

stilianouly commented Jul 4, 2018

Summary

This is probably related to your changes in 83f9323

When upgrading from 4.2.0 -> 4.3.0 one of our existing helper functions started exhibited strange behavior.

We have the following function to give us more readable messages when testing whether a field does not exist in some Json.

  def notHaveField(fieldName: String): Matcher[String] =
    AnyMatchers.not(
      haveField(fieldName).updateMessage(status => s"status: $status, field: $fieldName")
    ).updateMessage(selectorMatcherMessage => s"not($selectorMatcherMessage)")

The following test passes in 4.2.0, but now fails in 4.3.0

"not match when field is present" in {
  """{"a": 12}""" must  not(notHaveField("a"))
}

Workaround

We think it has something to do with .updateMessage on a JsonSelectorMatcher. When we re-write notHaveField to not update the JsonSelectorMatcher message, the test passes.

 def notHaveField(fieldName: String): Matcher[String] =
   AnyMatchers.not(
     haveField(fieldName)
   ).updateMessage(selectorMatcherMessage => s"not(field: $fieldName)")

The only difference is that we don't have access to the status which we don't really care about.

Is this the expected behavior of updateMessage?

@stilianouly stilianouly changed the title *updateMessage* on *JsonSelectorMatcher* yields unexpected results updateMessage on JsonSelectorMatcher yields unexpected results Jul 4, 2018
@etorreborre
Copy link
Owner

I think what changed is the meaning of "have". I suggest this alternative:

  def notHaveField(fieldName: String): Matcher[String] =
    AnyMatchers.not(
      /(fieldName -> anyValue).updateMessage(msg => msg.replace("specified matcher", "any value"))
    )

@stilianouly
Copy link
Author

I'm not sure I understand Eric.

I don't think the meaning of have would have changed, or should have changed.

Let me give a different example:

Test A (Passes)

"not match when field is present" in {
  """{"a": 12}""" must  not(not(haveField("a")))
}

Test B (Fails)

"not match when field is present" in {
  """{"a": 12}""" must  not(not(haveField("a").updateMessage(_=>"foo")).updateMessage(_=>"bar"))
}

The only difference between Test A and Test B is that Test B has multiple updateMessage() calls in the matcher stack. Test A passes, but Test B fails.

By updating multiple messages in the matcher stack, it seems to break the double negative principle.

I feel that changing the message of a matcher error shouldn't impact how that matcher behaves logically in the stack.


Curiously if only one message is updated in the stack, the specs seem to be fine.

Test C (Passes)

"not match when field is present" in {
  """{"a": 12}""" must  not(not(haveField("a")).updateMessage(_=>"bar"))
}

Test D (Passes)

"not match when field is present" in {
  """{"a": 12}""" must  not(not(haveField("a").updateMessage(_=>"foo")))
}

I'm not confident enough to go rummaging through the specs2 source, this is just something strange I noticed that I wanted to bring to your attention 😄

@etorreborre
Copy link
Owner

You are right, this is actually an issue with not and thrown failure exceptions which I didn't see because I was using an immutable spec. This is fixed now on master, do you need a release soon?

@stilianouly
Copy link
Author

We're OK with our workaround, Eric 👍

No rush to push a release.

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