allOf

[ijuma]

Hi all,

I was updating our tests to use `allOf` instead of our own custom matcher and ran into an issue. `allOf` takes a single parameter Any*. This is problematic. Both of the following compile, but only one of them works:

list should contain allOf(splitList)

list should contain allOf(splitList: _*)

This seems quite error-prone. I think the parameter should be a Traversable instead of varargs even if it's a bit more verbose in some cases.

Best,

Ismael

[Bill Venners]

Hi Ismael,

That sounds more like an allElementsOf. The allOf/only/etc. constructs all take varargs except for theSameElementsAs and theSameElementsInOrderAs. The word "elements" in there is a cue it will be looking inside a collection. So the consistently named construct would be:

list should contain allElementsOf(splitList)

Do you think something like that would help? I'll want to think about it, but even though "allOf(splitList: _*)" is fewer characters than "allElementsOf(splitList)", I might rather see the latter in code. But it might mean adding several more, which would add some clutter:

allElementsOf

noElementsOf

atLeastOneElementOf

atMostOneElementOf

onlyElementsOf

I'm not sure how I'd name one for inOrder and inOrderOnly that takes a collection, maybe "inOrderAllElementsOf" and "inOrderOnlyElementsOf" I suppose. But those are getting close to being non-obvious in meaning.

About the general problem, I think the fact that it takes an Any* bit you when you put the wrong type in there, because it will compile. You don't find out until the test fails. That's actually what the type-checked === addresses for equality checks, but in the case of contain matcher expressions, I ended up reducing the type checking to address this "feature" of ScalaTest 1.x:

scala> import org.scalatest._
import org.scalatest._

scala> import Matchers._
<console>:10: error: not found: value Matchers
       import Matchers._
              ^

scala> import matchers.ShouldMatchers._
import matchers.ShouldMatchers._

scala> List("1", 2, 3.0) should contain (2)
<console>:14: error: overloaded method value should with alternatives:
  (notWord: org.scalatest.matchers.ShouldMatchers.NotWord)org.scalatest.matchers.ShouldMatchers.ResultOfNotWordForSeq[Any,List[Any]] <and>
  (haveWord: org.scalatest.matchers.ShouldMatchers.HaveWord)org.scalatest.matchers.ShouldMatchers.ResultOfHaveWordForSeq[Any] <and>
  (beWord: org.scalatest.matchers.ShouldMatchers.BeWord)org.scalatest.matchers.ShouldMatchers.ResultOfBeWordForAnyRef[List[Any]] <and>
  (rightMatcher: org.scalatest.matchers.Matcher[List[Any]])Unit
 cannot be applied to (org.scalatest.matchers.Matcher[Traversable[Int]])
              List("1", 2, 3.0) should contain (2)
                                ^

To get that to compile in 1.x requires a type ascription, like:

scala> List("1", 2, 3.0) should contain (2: Any)

No terrible, but it did seem to me in the end that this should just compile and work:

List("1", 2, 3.0) should contain (2)

Because of the contravariance of matchers, which is correct, I could find no way of getting that to compile without essentially treating the right side as an Any. It fit in with allowing those to be customized with an implicit Equality as well, because the areEqual method of Equality takes an Any on the right hand side, which in turn was something I could also not find a way to get rid of for several reasons.

What is your thoughts on allElementsOf and constructs like that? It wouldn't solve the problem of accidentally using allOf incorrectly and not finding out until runtime, but I wonder if it might make it less error prone since users might get trained to include "Elements" in there if they want it to look inside a collection.

Bill

--
You received this message because you are subscribed to the Google
Groups "scalatest-users" group.
To post to this group, send email to scalate...@googlegroups.com
To unsubscribe from this group, send email to
scalatest-use...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/scalatest-users?hl=en
ScalaTest itself, and documentation, is available here:
http://www.artima.com/scalatest
---
You received this message because you are subscribed to the Google Groups "scalatest-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scalatest-use...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

--
Bill Venners
Artima, Inc.
http://www.artima.com

[ijuma]

Hi Bill,

Comments below.

On Wednesday, 4 September 2013 17:53:11 UTC+1, Bill Venners wrote:

Do you think something like that would help?

I think it would help, but I am not sure if this is the way to go. More below.

 

About the general problem, I think the fact that it takes an Any* bit you when you put the wrong type in there, because it will compile. You don't find out until the test fails.

This is indeed my main problem. The way it works is not what a Scala developer would expect. Consider the scenario where one has something like:

list should contain allOf("1", "2")

Then one extracts the parameters into a val/def so that it can be reused:

list should contain allOf(extracted)

Compiles fine, but then fails with an error that is hard to understand. One looks in the message and both the left and right seem to match. The person is now very confused. It's only when you look very carefully that you notice that there is some undesirable nesting on the right side. Please take a look at the error message when you make a mistake like this and see if it's obvious what is going on.

I would still prefer a compiler error (or for it to just work), but if we can't get that, can we please work out a way to make it easier for people to notice what the problem is?

scala> List("1", 2, 3.0) should contain (2)

This seems like a very edge-case though, right (In my 5 years of Scala, I have never had that need)? It's good to support, but we should not compromise common cases for edge cases.

What is your thoughts on allElementsOf and constructs like that? It wouldn't solve the problem of accidentally using allOf incorrectly and not finding out until runtime, but I wonder if it might make it less error prone since users might get trained to include "Elements" in there if they want it to look inside a collection.

I think `allElementsOf` and similar would be useful if we can't fix `allOf` and similar. I would prefer the latter if we can achieve it.

Ismael

[Bill Venners]

Hi Ismael,

We can definitely improve the error message for starters. It says:

scala> val list = List(1, 2, 3, 4, 5)
list: List[Int] = List(1, 2, 3, 4, 5)

scala> list should contain allOf(1, 2)

scala> val extracted = List(1, 2)
extracted: List[Int] = List(1, 2)

scala> list should contain allOf(extracted)
org.scalatest.exceptions.TestFailedException: List(1, 2, 3, 4, 5) did not contain all of (List(1, 2))

It would be more obvious if it said:

List(1, 2, 3, 4, 5) did not contain (List(1, 2))

Or perhaps we should disallow allOf (single-element vararg). Really for one element you should just use contain (element):

list should contain (singleElement)

Either way the error message would be clearer.

We couldn't really fall back to a backup approach if there's just one element passed and it is an Iterable. You might think we could just iterate through its elements and try again. Trouble with that is that if they really meant that the left hand side should contain as an element the right hand collection, then that's another potential error. We could possibly do this and then give a better error message, saying something like "List(1, 2, 3, 4, 5) did contain all *elements* of List(1, 2), but did not contain List(1, 2). Did you forget to write : _* or mean to use allElementsOf instead?"

By the way, this was the report of the overprotective type checking issue with contain:

http://code.google.com/p/scalatest/issues/detail?id=13

Bill

--

You received this message because you are subscribed to the Google
Groups "scalatest-users" group.
To post to this group, send email to scalate...@googlegroups.com
To unsubscribe from this group, send email to
scalatest-use...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/scalatest-users?hl=en
ScalaTest itself, and documentation, is available here:
http://www.artima.com/scalatest
---
You received this message because you are subscribed to the Google Groups "scalatest-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scalatest-use...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Bill Venners]

Hi Ismael,

I just had an idea for how to make this a compiler error. We could require at least two arguments to be passed to allOf like this:

scala> def allOf(firstEle: Any, secondEle: Any, remainingEles: Any*) = ()

allOf: (firstEle: Any, secondEle: Any, remainingEles: Any*)Unit

scala> allOf(List(1, 2))
<console>:18: error: not enough arguments for method allOf: (firstEle: Any, secondEle: Any, remainingEles: Any*)Unit.
Unspecified value parameters secondEle, remainingEles.
              allOf(List(1, 2))
                   ^

scala> allOf(1, 2)

In thinking about it, I think it would be good to offer allElementsOf-like options. We would not do this for the 2.0 release, but could do it after. I will likely mean I rename one of the existing ones for consistency, but can do that through a long deprecation period.

Also, I wanted to mention that another way you could skin your cat is:

scala> import org.scalatest._
import org.scalatest._

scala> import Matchers._

import Matchers._

scala> val list = List(1, 2, 3, 4, 5)
list: List[Int] = List(1, 2, 3, 4, 5)

scala> list should contain allOf(1, 2)

scala> val extracted = List(1, 2)
extracted: List[Int] = List(1, 2)

scala> import Inspectors._
import Inspectors._

scala> forAll (extracted) { list should contain (_) }

Bill

[ijuma]

On Friday, 6 September 2013 00:17:12 UTC+1, Bill Venners wrote:

I just had an idea for how to make this a compiler error. We could require at least two arguments to be passed to allOf like this:

scala> def allOf(firstEle: Any, secondEle: Any, remainingEles: Any*) = ()

allOf: (firstEle: Any, secondEle: Any, remainingEles: Any*)Unit

scala> allOf(List(1, 2))
<console>:18: error: not enough arguments for method allOf: (firstEle: Any, secondEle: Any, remainingEles: Any*)Unit.

Nice. Although, it means that there wouldn't be a way to pass a collection for my original use-case until the `allElementsOf`-like version shows up in a subsequent release. Not the end of the world, I can add back the matcher we were using for this situation.

 

In thinking about it, I think it would be good to offer allElementsOf-like options. We would not do this for the 2.0 release

Yes, it would be good to have this indeed.

Ismael