Include Rex's collections-laws tests in PR validation

[SethTisue]

see discussion at scala/scala#5891 (comment)

where e.g. @Ichoran wrote:

You can try running collections-laws for a start, which ought to catch these sorts of things, and says how good its coverage is. If it doesn't catch stuff like this, we should add a law. We also should include it in the automatic test suite before release, instead of having me intermittently running it manually. But I'm making it more friendly for other contributors (by making more of it Scala code instead of doing weird text replacement off of files with unique syntax), so maybe not just yet.

[SethTisue]

note that the current scala-collections-laws code is in @Ichoran’s fork, not (yet?) in the main repo under the scala org

the 2.12 community build includes a branch of that fork

[Ichoran]

It's there (in my repo only) because it's a work in progress. There are still collections that aren't covered, and methods that aren't covered, and the way to invoke it is pretty clunky. (Shell script calling two different sbt tasks.)

[SethTisue]

thanks, I'll try that branch.

the 2.12 community build includes a branch of that fork

as I look at this again for the first time in a long time, I note that the community build is just making sure that the repo compiles on latest Scala, it isn't running the full arsenal of tests. we need to add that to the Scala release steps.

[SethTisue]

note that the current scala-collections-laws code is in @Ichoran’s fork, not (yet?) in the main repo under the scala org

well that much is fixed for 2.12 via scala/scala-collection-laws#18 and scala/community-build@65a4be2

[SethTisue]

scala/community-build@a7a8a73 adds Rex's 2.13 branch to the 2.13 community build, where it is green. (but as with 2.12 as mentioned earlier, that just means it compiles, not that the tests pass)

[SethTisue]

got the scala-collections-laws 2.13.x branch running on my Mac. as the readme at https://github.com/Ichoran/scala-collections-laws/tree/only-new-coll-2.13 says, you have to publishLocal lihaoyi/sourcecode first, otherwise easy.

it takes about 10 minutes to run (faster than I'd expected/feared). there are currently a number of "Untested methods" warnings and a number of failures, more than a few, but not an overwhelming amount.

as an experiment, I did bash run.sh > out.txt against 2.13.0-pre-53c4be1 (the current M5 release candidate, unless we decide to rebuild), then I added VectorMap to Instantiator.scala and Generator.scala (copying the immutable.ListMap lines) and did bash run.sh > out2.txt, then inspected the output of diff -u out.txt out2.txt

I see

+Untested methods in ImmKV_VectorMap:
+  addString         aggregate         default           filterKeys        
+  flatten           fold              groupMapReduce    keySet            
+  keys              keysIterator      lazyZip           mapFactory        
+  product           reduceLeft        reduceLeftOption  reduceOption      
+  reduceRight       reduceRightOption remove            removeAll         
+  repr              scanRight         values            valuesIterator    
+  withDefault       withDefaultValue  withFilter        

which is similar (identical?) to what we see for the other maps, so no surprise there

there's a lot of further output that it's hard to fully evaluate just by eyeballing, but I did spot:

out2.txt:Fail((x.`-`(a._1)).`count`(_ == a) == 0
out2.txt:Fail((x.`-`(a._1)).`count`(_ == a) == 0
out2.txt:Fail((x.`-`(a._1)).`size` == x.size - (if (x.`contains`(a._1)) 1 else 0)
out2.txt:Fail((x.`-`(a._1)).`size` == x.size - (if (x.`contains`(a._1)) 1 else 0)
out2.txt:Fail((x.`-`(a._1)) partOf x
out2.txt:Fail((x.`-`(a._1)) partOf x
out2.txt:Fail(x.`size` > 0 implies (x.`init` sameAs x.`dropRight`(1))
out2.txt:Fail(x.`size` > 0 implies (x.`init` sameAs x.`dropRight`(1))
out2.txt:Fail(x.`size` > 0 implies (x.`init` partOf x)
out2.txt:Fail(x.`size` > 0 implies (x.`init` partOf x)
out2.txt:Fail(xsize < 1 || { x.`tail` sameAs x.`drop`(1) }
out2.txt:Fail(xsize < 1 || { x.`tail` sameAs x.`drop`(1) }

which shows that the laws already catch all three of the bugs in scala/bug#11100, which is the ticket that brought our attention back to this ticket here. yay scala-collections-laws!

finally, I ran it again with the Scala built from scala/scala#7123 , with @mdedetrich's and my VectorMap fixes, and verified that those failures go away, as expected

anyway, I wanted to mess with this stuff and get a bit more familiar with it, mission somewhat accomplished.

[SethTisue]

anyway, clearly we need to get this stuff in better shape in time for 2.13.0-RC1.

the title of this ticket suggests making scala-collections-laws part of scala/scala PR validation. that seems ideal, but an easier, shorter-term goal would be for the Scala 2.13 community build to actually run these tests.