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

Cleanup fx tests #90

Merged
merged 9 commits into from
Mar 10, 2020
Merged

Cleanup fx tests #90

merged 9 commits into from
Mar 10, 2020

Conversation

danimontoya
Copy link
Contributor

@danimontoya danimontoya commented Mar 6, 2020

Continues the work done in #51 :
Removes a bunch of duplicate code.

This fails building with gradle locally because it can't find the generators for some reason. Intellij fails to shows this as an error and I don't even understand why it'd fail^^

Edit: I also changed arrow-fx-test to be published the same way as arrow-core-test

@1Jajen1
Copy link
Member

1Jajen1 commented Mar 6, 2020

Is this a redo of #51 ? If so I'll close that and stop trying to figure out what was broken since this one actually compiles^^ 👍

@danimontoya
Copy link
Contributor Author

Yes sorry I did not tell you yet.. but was being hard to make it work and updating #51 😅
I'll ping you when its done to have a look! thanks!

@1Jajen1
Copy link
Member

1Jajen1 commented Mar 7, 2020

Yes sorry I did not tell you yet.. but was being hard to make it work and updating #51 sweat_smile
I'll ping you when its done to have a look! thanks!

No problem, just got a bit confused. Glad to see this compiling^^

@1Jajen1 1Jajen1 mentioned this pull request Mar 7, 2020
Copy link
Member

@nomisRev nomisRev left a comment

Choose a reason for hiding this comment

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

Great work @danimontoya 👏 👏 Happy to see it green! 🎉

I left some comments in regard to where things can be moved to give them more meaningful locations so people can also depend on them if they want. arrow-fx-test will also make it more worthwhile to write better utilities that can be shared.

Schedule and Resource also have Eq, EqK, Gen, GenK and Gen2 implementations that can be moved to arrow-fx-test.

}
}

fun <F, A> EQ(EQ: Eq<Kind<F, A>>): Eq<FiberOf<F, A>> = object : Eq<FiberOf<F, A>> {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fun <F, A> EQ(EQ: Eq<Kind<F, A>>): Eq<FiberOf<F, A>> = object : Eq<FiberOf<F, A>> {
fun <F, A> Fiber.Companion.eq(EQ: Eq<Kind<F, A>>): Eq<FiberOf<F, A>> = object : Eq<FiberOf<F, A>> {

We can also add an EqK instance for Fiber.

gen.map(IO.Companion::just),
Gen.throwable().map(IO.Companion::raiseError)
)
}
Copy link
Member

Choose a reason for hiding this comment

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

GenK of Fiber can also be moved here

}
fun <A, B> Gen.Companion.functionABToB(gen: Gen<B>): Gen<(A, B) -> B> = gen.map { b: B -> { _: A, _: B -> b } }

fun <A> Gen<A>.genEval(): Gen<Eval<A>> =
Copy link
Member

Choose a reason for hiding this comment

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

Eval comes from arrow.core.

import io.kotlintest.properties.Gen
import io.kotlintest.should
import io.kotlintest.shouldNot

// TODO: check to move to arrow-core-test
fun throwableEq() = Eq { a: Throwable, b ->
Copy link
Member

Choose a reason for hiding this comment

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

I am okay leaving this in arrow-fx-test since effects imply Throwable, and comparing Throwable can be dependent on use-case. Let's move this with the other Eq instances tho.

I think this is a fair implementation to leave public in arrow-fx-test, but we should add a clear KDoc that comparing Throwable is not safe due to their structure (stacktrace), so we structurally compare type and message instead.

override fun FiberOf<ForIO, Int>.eqv(b: FiberOf<ForIO, Int>): Boolean = EQ<Int>().run {
fix().join().eqv(b.fix().join())
}
}

fun EQK() = object : EqK<FiberPartialOf<ForIO>> {
Copy link
Member

Choose a reason for hiding this comment

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

See other comment, move to arrow-fx-test.

compile "io.arrow-kt:arrow-mtl:$VERSION_NAME", excludeArrow
compile "io.arrow-kt:arrow-mtl-data:$VERSION_NAME", excludeArrow
compile "io.arrow-kt:arrow-free:$VERSION_NAME", excludeArrow
Copy link
Member

@nomisRev nomisRev Mar 7, 2020

Choose a reason for hiding this comment

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

Suggested change
compile "io.arrow-kt:arrow-free:$VERSION_NAME", excludeArrow

We don't need free in arrow-fx-test


dependencies {
compile project(":arrow-fx")
compile "io.arrow-kt:arrow-core-test:$VERSION_NAME", excludeArrow
api "io.arrow-kt:arrow-core-test:$VERSION_NAME", excludeArrow
compile "io.arrow-kt:arrow-mtl:$VERSION_NAME", excludeArrow
Copy link
Member

Choose a reason for hiding this comment

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

We don't need mtl in arrow-fx-test

compile "io.arrow-kt:arrow-free:$VERSION_NAME", excludeArrow
compile "io.arrow-kt:arrow-free-data:$VERSION_NAME", excludeArrow
compile "io.arrow-kt:arrow-recursion-data:$VERSION_NAME", excludeArrow
compile "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$KOTLIN_VERSION"
compile "org.jetbrains.kotlinx:kotlinx-coroutines-core:$KOTLINX_COROUTINES_VERSION"
Copy link
Member

Choose a reason for hiding this comment

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

Created #93 for the removal of this dependency. This is low priority so not blocking for 0.10.x

@danimontoya danimontoya changed the title temporary draft cleanup fx tests Cleanup fx tests Mar 9, 2020
…nflicting with it

Move EQK and GENK from FiberTest to EqK and GeneratorsFx
Clean up dependencies not used
@danimontoya danimontoya marked this pull request as ready for review March 9, 2020 23:12
Copy link
Member

@rachelcarmena rachelcarmena left a comment

Choose a reason for hiding this comment

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

Awesome job!! 🎉

@rachelcarmena rachelcarmena merged commit 99c6226 into master Mar 10, 2020
@rachelcarmena rachelcarmena deleted the dm-fx-tests-cleanup branch March 10, 2020 07:47
@nomisRev
Copy link
Member

Unblocks arrow-kt/arrow-incubator#63

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