Improve shrinking of Command sequences and allow customization

[jonaskoelker]

There are three improvements here. Together they accomplish more than
the sum of their parts.

• Instead of shrinking the State with shrinkWithOrig and combining
  the result with the shrunk sequential and parallel commands, just
  shrink each Actions field independently. That is, include in the
  shrink output some Actions where only the State has been shrunk;
  previously State-shrinking would only occur together with shrinking
  of at least one command sequence.

• Instead of throwing out Actions objects that do not satisfy the
  actionsPrecond predicate, throw out individual Command objects to
  make the Actions object conform to actionsPrecond. This allows
  the state and commands to be shrunk jointly in a sensible fashion.

• Allow the user to customize command shrinking, at three levels:

  • shrinkCommand shrinks individual commands.
  • shrinkSequentialCommands shrinks the sequential part.
  • shrinkParallelCommands shrink the parallel command sequences.
    If the user only defines shrinkCommand, the other two will be
    defined using implicitly for List.

[jonaskoelker]

As an example where the changes I'm suggesting here really help, see https://gist.github.com/jonaskoelker/ac60ded3a310b8f0fb548a1eda9f8859 and https://gist.github.com/jonaskoelker/b4a1d00a3da97c6d56e7b3d8fee02716 (though you will need to add .retryUntil(cmd => cmd.preCondition(state)) to the genCommand result to succeed at running the tests).

Try removing all the implementations of the bounded queue methods, replacing them with ???, then add the lines back in 1-3 at a time (heavy TDD style) to address each failing test case. Compare the counterexamples before and after this commit. Note in particular that with this commit, all counterexample queues have capacity less than 10 (the smallest capacity with which they're generated).

[jonaskoelker]

I see Travis complaining that the methods I added only exist after the patch and not before, in the context of Failed binary compatibility check against org.scalacheck:scalacheck_sjs0.6_2.11:1.14.3. I think the thing to do about that is in part a function of your release process which I'm not aware of.

One thing that can easily be done is to split this PR into two, where one does everything except add the customization option, and the other adds the customization option. If the only complaints about binary compatibility are about the public methods I've added, I have a hunch that moving those into a separate PR should let everything else go through. That of course just narrows the problem without fixing anything. But maybe it'll enable a binary-compatible minor-version release with some of these changes? (Assuming you want them in the first place, of course.)

[ashawley]

Yeah, there is a tool that is flagging that there is a binary compatability since Commands isn't marked final. It's strange that it's only happening for Scala 2.11.

[error] scalacheck: Failed binary compatibility check against org.scalacheck:scalacheck_2.11:1.14.3! Found 3 potential problems (filtered 2)
[error]  * method shrinkCommand()org.scalacheck.Shrink in trait org.scalacheck.commands.Commands is present only in current version
[error]    filter with: ProblemFilters.exclude[ReversedMissingMethodProblem]("org.scalacheck.commands.Commands.shrinkCommand")
[error]  * method shrinkSequentialCommands()org.scalacheck.Shrink in trait org.scalacheck.commands.Commands is present only in current version
[error]    filter with: ProblemFilters.exclude[ReversedMissingMethodProblem]("org.scalacheck.commands.Commands.shrinkSequentialCommands")
[error]  * method shrinkParallelCommands()org.scalacheck.Shrink in trait org.scalacheck.commands.Commands is present only in current version
[error]    filter with: ProblemFilters.exclude[ReversedMissingMethodProblem]("org.scalacheck.commands.Commands.shrinkParallelCommands")

You can fix by adding the following to project/MimaSettings.scala:

  private def newMethods = Seq(
    "org.scalacheck.commands.Commands.shrinkCommand",
    "org.scalacheck.commands.Commands.shrinkSequentialCommands",
    "org.scalacheck.commands.Commands.shrinkParallelCommands",
  )

[SethTisue]

Is there anyone watching this repo who feels able to assess/review this...?

[ashawley]

One thing that can easily be done is to split this PR into two

That would probably be the best thing to do. There are fewer experts on the commands API still around, so reducing the complexity in this part of the library would more likely get your PRs merged.

I know tests of shrinking is difficult and there aren't many existing commands tests, but if you can add tests, that would probably help, as well.

Sorry for the delay. I didn't know you had fixed MiMa.

[jonaskoelker]

I've created two new PRs (#739 and #740) which address two of the parts of this PR. The third part is guaranteed to conflict with both of those PRs, so I suggest I create the last split-PR once the other two have been landed.