let Mapk 'zipN with nullable' tests use Arb.map2 to generate values

[abendt]

This PR builds on #2980

there are a couple of MapK tests that test zipN with nullables. The tests construct the required testdata in the test body from the passed in arguments.

The PR #2980 has introduced an Arb that can be used to generate Maps that share a number of keys. However this Arb did not yet support null values.

In this PR the mentioned Arb is modified so it can also be used to generate Maps that have null values. This Arb is then used in the 'zipN with nullable' tests to replace the testdata setup in the test body.

#2980 (comment)
@serras I have added a fixed RandomSource to ensure the tests for the Arb won't be flaky.

[serras]

I think the change in the width of the integers and lists is making tests to take much longer in Kotlin/Native targets. Could we keep the maps under 10 keys, and intSmall, as done in the code that this replaces? Or otherwise, find another way to make tests run faster.

[abendt]

apparently some of the GeneratorTests took a long time. I changed them so they run faster now.

[serras]

@abendt thanks for the hard work on this! I'm a bit worried still by the fact that Native tests seem to have gone much slower (by comparison, other PRs take around 25 minutes on Mac, and 20 on Windows, and this one takes almost double).

[abendt]

@serras sure, will have another look and then get back to you

[abendt]

Hi @serras, here are my results

To compare the times i picked this PR (#2993). This should be a PR that comes right before any of my MapKTest related changes.

This are the overall build times + time to execute MapKTest (from respective gradle build scan)

https://github.com/arrow-kt/arrow/actions/runs/4511556481

iOS 41m 13s
https://scans.gradle.com/s/k7hnxy5oj5aq4
MapKTest 35s

macOS 32m 47s
https://scans.gradle.com/s/6xbjuwiolceaw
MapKTest 31s

Windows 28m 48s
https://scans.gradle.com/s/zx5rh2xyxfvjy
MapKTest 1m7s

This are the times with the MapKTest related changes

https://github.com/arrow-kt/arrow/actions/runs/4542439780

iOS 53m 18s
https://gradle.com/s/gbryzg6wz6yac
MapKTest 8m 12s (Note here that this build executes the MapKTest multiple times for different mpp targets, i have just listed one occurence)

macOS 45m 59s
https://gradle.com/s/loo7geadhtbq6
MapKTest 6m 42s

Windows 27m 41s
https://gradle.com/s/5h66f4wqmgj2e
MapKTest 9m 37s

As you can see the test times for MapKTest increased dramatically. The reason for that seems to be the new Arb i have implemented. Internally it uses kotests Arb.map(...) (here https://github.com/arrow-kt/arrow/blob/main/arrow-libs/core/arrow-core/src/commonTest/kotlin/arrow/core/test/Generators.kt#L196)

On my machine i could see that running with the gradle iosSimulatorArm64Test it takes about 10seconds to generate 250 maps with the mentioned kotest function. With jvmTest the same task just takes 700ms.

This should explain the increase in duration as there are a bunch of new tests in MapKTest that use this generator.

To improve the situation i have
Limited the size of generated maps to 20 (

arrow/arrow-libs/core/arrow-core/src/commonTest/kotlin/arrow/core/test/Generators.kt

Line 235 in 8d002f6

Arb.map(keyArb = arbK, valueArb = value2(arbA, arbB), minSize = size.first, maxSize = size.last)

)

These are the test times after the mentioned change

https://github.com/arrow-kt/arrow/actions/runs/4545366194

iOS 43m 50s
https://gradle.com/s/3oslncwg4grxo
MapKTest 3m 6s

macOS 39m 33s
https://gradle.com/s/x2hquhkbtxecw
MapKTest 2m 44s

Windows 25m 20s
https://gradle.com/s/brdhrgifjdrt2
MapKTest 4m 24s

@serras do you think the increase in time is acceptable now?

for reference this is the code snippet i used to measure the Arb

"measure" {

    println("Map<A,B>")
    measure(Arb.map(Arb.string(), Arb.int()))

    println("Map2<String, Boolean, Boolean>, 100")
    measure(Arb.map2(Arb.string(), Arb.boolean(), Arb.boolean(), 0 .. 100))

    println("Map2<String, Boolean, Boolean>, 50")
    measure(Arb.map2(Arb.string(), Arb.boolean(), Arb.boolean(), 0..50))

    println("Map2<String, Boolean, Boolean>, 20")
    measure(Arb.map2(Arb.string(), Arb.boolean(), Arb.boolean(), 0..20))
  }

...

private fun <A> measure(arb: Arb<A>, count: Int = 1000) {
  val random = RandomSource.seeded(0)
  val time = measureTime {
    arb.generate(random).take(count).forAll {
      it.shouldNotBeNull()
    }
  }

  println("Total: " + time.inWholeMilliseconds)
  println("Took: " + time.inWholeMilliseconds.toDouble() / count)
}

[abendt]

i tried out different types of maps (iosSimulatorArm64Test):

    Map<String,Int>
    Total: 10736
    Took: 10.736
    Map<Int,Int>
    Total: 148
    Took: 0.148
    Map<Short,Int>
    Total: 208
    Took: 0.208

So i have changed the code to use Arb.map(Arb.int(), Arb.int()) in MapKTest

This further seems to improve the times:

windows

• https://scans.gradle.com/s/4kwv5txrvo23y
• 24m 37s
• MapKTest 28,69s

macos

• https://scans.gradle.com/s/p7xcw4ir6yrm4
• 36m
• MapKTest 17,2 s

So i think i can remove the size limit again and we can go just with the change of types

[serras]

Great job improving our tests, @abendt! Especially because now we know that properties are really covered by the data being produced.

I want to add that I'm impressed by the word you've put into this, involving several rounds of pushing to GitHub and looking at the results of the actions. Thank you!

[nomisRev]

Amazing work @abendt 🙌 👏 🥳