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

Make Gen[Tuple] serializable and add regression test #672

Closed
wants to merge 2 commits into from

Conversation

yhylord
Copy link
Contributor

@yhylord yhylord commented Jul 17, 2020

Right now, tuple generators (e.g. Gen[(String, Int)]) are not serializable. Generated functions that have tuples in the result (e.g. List[Int] => List[(String, Int)]) also don't serialize.
This PR adds serializability to the mentioned values. It does so by removing Function1 which is not serializable and use flatMap and map to generate zip code in codegen.scala. A serialization regression test is also included.

@non
Copy link
Contributor

non commented Jul 18, 2020

@yhylord It looks like these instances are serializable on 2.13 but are not serializable on 2.11 or 2.12 on the JVM. (We're not running these tests on the JS platform which is why that's OK.)

The change does seem to help 2.13; it's possible for 2.11 and 2.12 that we'd need to create our own serializable Function1 subclass to get this working.

@non
Copy link
Contributor

non commented Jul 18, 2020

@yhylord You can choose to build/test Scala 2.12.10 (instead of 2.13) by running ++2.12.10 from SBT before your other commands. For example:

$ sbt
... various startup output of SBT ...
sbt:scalacheck> ++2.12.10
[info] Setting Scala version to 2.12.10 on 4 projects.
[info] Excluded 1 projects, run ++ 2.12.10 -v for more details.
[info] Reapplying settings...
[info] set current project to scalacheck (in build file:/Users/erik/w/scalacheck/)
sbt:scalacheck> jvm/compile
[info] Compiling 9 Scala sources to /Users/erik/w/scalacheck/jvm/target/scala-2.12/classes ...
[info] Compiling 8 Scala sources to /Users/erik/w/scalacheck/jvm/target/scala-2.12/classes ...
[success] Total time: 20 s, completed Jul 17, 2020 9:13:27 PM
sbt:scalacheck> jvm/test
... test output follows ...

The command is stateful so it will last until you restart SBT. You can use different Scala versions there (e.g. 2.11.12 to test on 2.11 as well).

Base automatically changed from master to main March 26, 2021 18:56
@larsrh larsrh closed this in #795 Apr 16, 2021
@larsrh
Copy link
Contributor

larsrh commented Apr 16, 2021

Rebased & merged in #795.

@ashawley
Copy link
Contributor

To be honest, the commit in #795 did credit Haoyang Yu, but it was slightly modified from the original.

@ashawley
Copy link
Contributor

The discussion of tuple serialization above last year suggested that this would work on 2.13, but not 2.12. I couldn't find that to be the case. I took the change made here and added some test cases to the existing serialization suite and created #795. It worked fine for both Scala versions on the JVM, fwiw. I'm not sure what changed since last year. It's not clear what's different about Function1 from using Scala's function syntax, but it seemed like the right change going forward, regardless.

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

Successfully merging this pull request may close these issues.

5 participants