Skip to content
This repository has been archived by the owner on Feb 24, 2021. It is now read-only.

Reduce test stress #190

Merged
merged 14 commits into from
Jun 22, 2020
Merged

Reduce test stress #190

merged 14 commits into from
Jun 22, 2020

Conversation

nomisRev
Copy link
Member

Reduced the stress of our test suite by 10x by reducing the iterations to 100 instead of 1000.

This has shaved off ~ 7minutes on my local machine, while still covering all edgeCases.
There are two implementations here based on Kotest in Arrow Fx Coroutines, and KotlinTest in the rest.
We should push this change down to Arrow Core Test, so we can also configure iterations for law testing.

Proposed follow up work

Ideally, we'd decouple property testing from Kotest so:

-fun <A> Monoid<A>.monoidLeftIdentity(GEN: Gen<A>, EQ: Eq<A>): Unit =
+fun <A> Monoid<A>.monoidLeftIdentity(EQ: Eq<A>): Boolean =
-  forAll(GEN) { a ->
    (empty().combine(a)).equalUnderTheLaw(a, EQ)
-  }

This allows for more flexibility as you could plug it into both KotlinTest & Kotest and that simply becomes a integration instead of hard-coupled as it is today.

This also decouples the property testing from the law definition, and would allow us to easily control test strategy by the caller.

@JorgeCastilloPrz
Copy link
Member

I know this is a hard to answer question but, how many iterations are okay to ensure code integrity? Could 100 iterations be too low to test an acceptable amount of edge cases?

@nomisRev
Copy link
Member Author

@JorgeCastilloPrz Kotest checks that you have enough iterations to cover all edgeCases, so we're guaranteed to cover all edgeCases + iterations - edgeCases.size random values.

https://github.com/kotest/kotest/blob/master/kotest-property/src/commonMain/kotlin/io/kotest/property/internal/proptest.kt#L17

I just double checked for KotlinTest, and it doesn't have the same assurance but we need to refactor to Kotest asap since we're a version behind now.

Copy link
Member

@aballano aballano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this is compatible with arrow-kt/arrow-core#105 somehow?

@nomisRev
Copy link
Member Author

@aballano it's definitely combine-able with arrow-kt/arrow-core#105, but I feel like we should come up with a strategy that also incorporates upgrading to kotest.

@nomisRev nomisRev merged commit 464e896 into master Jun 22, 2020
@nomisRev nomisRev deleted the sv-reduce-test-stress branch June 22, 2020 18:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants