ZIO Test Integration #282

hearnadam · 2024-04-24T06:23:42Z

Working, albeit messy zio-test integration. I think this is generally preferable to Scalatest (for many reasons), but open to feedback.

Much of this is closely tied to the implementation of ZIOSpecDefault or more accurately ZIOSpec.

I currently cannot get sbt to detect the tests to run, but that may be a fault in my configuration.

One of the perks of Kyo's model is that we can simply assume all assertions may have pending effects. As such, we can always invoke the run. In ZIO's case, there is logic to determine which TestConstructor[_, _] to use via implicit resolution. If a pure TestResult is passed as an assertion, it's necessary to wrap it in ZIO.succeed

kyo-core/shared/src/test/scala/kyoTest/KyoTestZIO.scala

hearnadam · 2024-04-24T19:31:59Z

@fwbrasil please review when you get a chance. We can split this out into kyo-zio-test prior to publishing. I will also split out these into multiple files for each class.

hearnadam · 2024-04-24T19:42:43Z

kyo-core/shared/src/test/scala/kyoTest/KyoSpecAbstract.scala

+import zio.internal.stacktracer.SourceLocation
+import zio.test.*
+
+abstract class KyoSpecAbstract[S] extends ZIOSpecAbstract:


ZIOSpecAbstract has a timeout warning after 60 seconds. This works correctly with Fiber.sleep:

timestamp=2024-04-24T19:41:29.306520Z level=WARN thread=#zio-fiber-160508830 message="Test suite! - Fibers.sleep has taken more than 1 m to execute. If this is not expected, consider using TestAspect.timeout to timeout runaway tests for faster diagnostics."

I think we need to propagate ZIO's cancelation to Kyo's fibers otherwise the test could continue running in the background.

This is true, though may not be a problem for an initial implementation. ZIO uses .disconnect to provide immediate interruption of ZIO effects when TestAspect.timeout(..) is used. Given this is for testing, I don't think we should be too concerned about resource leaks... though that's likely a bad precedence to set.

We will need to find a suitable point of introp with ZIO which supports cancellation. ZIO has an CancelableFuture, but I can't find any info on it.

kyo-core/shared/src/test/scala/kyoTest/KyoSpecAbstract.scala

hearnadam · 2024-04-24T19:44:52Z

kyo-core/shared/src/test/scala/kyoTest/KyoSpecAbstract.scala

+
+abstract class KyoSpecDefault extends KyoSpecAbstract[KyoApp.Effects]:
+ final override def run[In](v: => In < KyoApp.Effects)(using Flat[In < KyoApp.Effects]): ZIO[Environment, Throwable, In] =
+ ZIO.fromFuture { implicit ec => IOs.run(KyoApp.runFiber(timeout)(IOs(v)).toFuture).map(_.get) }


Maybe we can add a private[kyo] def KyoApp.runToFuture method.

Note, I am wrapping in IOs here to ensure suspension - not sure that's necessary.

Maybe KyoApp.runFuture? It could be public as well, it seems useful. There's no need for the IOs suspension since the code handles IOs and is also suspended in ZIO.

Sure - I was just hesitant to suggest a public function as it seems like it could be misused.

hearnadam · 2024-04-24T19:46:49Z

kyo-core/shared/src/test/scala/kyoTest/KyoSpecAbstract.scala

+ final override def run[In](v: => In < KyoApp.Effects)(using Flat[In < KyoApp.Effects]): ZIO[Environment, Throwable, In] =
+ ZIO.fromFuture { implicit ec => IOs.run(KyoApp.runFiber(timeout)(IOs(v)).toFuture).map(_.get) }
+
+ def timeout: Duration = Duration.Inf


Not sure we need this. Users can use the ZIO timeout aspects.

A default timeout is important since people might not add the aspect. KyoTest has a behavior to disable the timeout in case there's an active debugging session. It's quite useful when working on tests so we'd need to port it as well.

Can we can update Fibers.timeout to ignore Duration.Inf? This should lower the overhead for each test

fwbrasil · 2024-04-24T20:22:25Z

I haven't used zio-test yet, could you share more on its advantages? Something to consider is if it'd impact the execution time of the tests. I'd be much more inclined to migrate in case it's faster for example. A fast test suite is essential in my development flow.

hearnadam · 2024-04-24T20:33:58Z

zio-test will likely be equivalent in performance to scalatest - though there are ways to speed it up. You can use the TestAspect.parallel - I don't remember scalatest having this feature. The best benefits of zio-test in my experience are:

test generators for property testing: https://zio.dev/reference/test/property-testing/how-generators-work/
easy to use assertion mechanism, with clear error rendering

In scalatest the following is a valid passing test:

val x = 1
val y = 2

x should be 
y

One final benefit is that zio-test is likely popular with many kyo users. If we expect users to interop with zio at all, it would be best if kyo-test ran on the same test runner.

fwbrasil · 2024-04-25T23:19:57Z

LGTM. Is this ready to merge?

hearnadam · 2024-04-25T23:21:11Z

@fwbrasil it can be merged in the current state, but the remaining work:

split to new project/module
split to distinct files
add KyoApp.runFuture / equivalent
add unit tests

hearnadam · 2024-04-26T01:59:05Z

@fwbrasil I removed the if debug for the fiber timeout, as I don't want to migrate the existing Platform implementation. I think we can use test aspects or another ZIO specific mechanism for this...

I split the code to new files project, so it should be consumable from all kyo projects. We can resolve the remaining issues later.

hearnadam added 2 commits April 23, 2024 23:14

WIP ZIO Test integration

4973fee

add test using zio timeout

01d6f63

hearnadam commented Apr 24, 2024

View reviewed changes

kyo-core/shared/src/test/scala/kyoTest/KyoTestZIO.scala Outdated Show resolved Hide resolved

hearnadam changed the title ~~Test integ~~ ZIO Test Integration Apr 24, 2024

Improve separation of abstract/default general cleanup

aab6f25

hearnadam marked this pull request as ready for review April 24, 2024 19:31

hearnadam commented Apr 24, 2024

View reviewed changes

add timeout remove IOs suspension

d819166

create new package kyo-test

e6add1f

fwbrasil merged commit 8f81fa0 into getkyo:main Apr 26, 2024
3 checks passed

hearnadam mentioned this pull request Apr 27, 2024

Test Support #214

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ZIO Test Integration #282

ZIO Test Integration #282

hearnadam commented Apr 24, 2024

hearnadam commented Apr 24, 2024

hearnadam Apr 24, 2024

fwbrasil Apr 24, 2024

hearnadam Apr 25, 2024

hearnadam Apr 24, 2024

fwbrasil Apr 24, 2024 •

edited

Loading

hearnadam Apr 24, 2024

hearnadam Apr 24, 2024

fwbrasil Apr 24, 2024 •

edited

Loading

hearnadam Apr 24, 2024

fwbrasil commented Apr 24, 2024

hearnadam commented Apr 24, 2024 •

edited

Loading

fwbrasil commented Apr 25, 2024

hearnadam commented Apr 25, 2024 •

edited

Loading

hearnadam commented Apr 26, 2024

ZIO Test Integration #282

ZIO Test Integration #282

Conversation

hearnadam commented Apr 24, 2024

hearnadam commented Apr 24, 2024

hearnadam Apr 24, 2024

Choose a reason for hiding this comment

fwbrasil Apr 24, 2024

Choose a reason for hiding this comment

hearnadam Apr 25, 2024

Choose a reason for hiding this comment

hearnadam Apr 24, 2024

Choose a reason for hiding this comment

fwbrasil Apr 24, 2024 • edited Loading

Choose a reason for hiding this comment

hearnadam Apr 24, 2024

Choose a reason for hiding this comment

hearnadam Apr 24, 2024

Choose a reason for hiding this comment

fwbrasil Apr 24, 2024 • edited Loading

Choose a reason for hiding this comment

hearnadam Apr 24, 2024

Choose a reason for hiding this comment

fwbrasil commented Apr 24, 2024

hearnadam commented Apr 24, 2024 • edited Loading

fwbrasil commented Apr 25, 2024

hearnadam commented Apr 25, 2024 • edited Loading

hearnadam commented Apr 26, 2024

fwbrasil Apr 24, 2024 •

edited

Loading

fwbrasil Apr 24, 2024 •

edited

Loading

hearnadam commented Apr 24, 2024 •

edited

Loading

hearnadam commented Apr 25, 2024 •

edited

Loading