-
Notifications
You must be signed in to change notification settings - Fork 80
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
Introduce alternative testing infrastructure to test single render passes more directly. #349
Conversation
ac47ed4
to
92584ef
Compare
@@ -28,7 +28,7 @@ import kotlinx.coroutines.Deferred | |||
* | |||
* @see RealRenderContext | |||
*/ | |||
internal data class Behavior<StateT, out OutputT : Any>( | |||
data class Behavior<StateT, out OutputT : Any> internal constructor( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sad to make this public, but don't want to duplicate the code in the test module.
*/ | ||
internal class RealRenderContext<StateT, OutputT : Any>( | ||
class RealRenderContext<StateT, OutputT : Any>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very sad to make this public. See above comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I brought this up in the Kotlin Slack. I should file a ticket: https://kotlinlang.slack.com/archives/C0B8PUJGZ/p1555962527006200
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you do, lemme know I will vote.
@@ -30,17 +30,21 @@ internal typealias AnyId = WorkflowId<*, *, *> | |||
* Value type that can be used to distinguish between different workflows of different types or | |||
* the same type (in that case using a [name]). | |||
*/ | |||
internal data class WorkflowId<in InputT, out OutputT : Any, out RenderingT> | |||
data class WorkflowId<in InputT, out OutputT : Any, out RenderingT> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😢
@@ -0,0 +1,20 @@ | |||
package com.squareup.workflow.testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copyright
val result = workflow.testRender() | ||
|
||
result.assertNoWorkflowsRendered() | ||
result.assertNoWorkersRendered() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: do we really render a worker? Don't we run them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spent a whole 5 minutes googling the correct tenses of "to run", I think I got it right now.
@@ -0,0 +1,144 @@ | |||
package com.squareup.workflow.testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copyright
92584ef
to
b29baf0
Compare
*/ | ||
internal class RealRenderContext<StateT, OutputT : Any>( | ||
class RealRenderContext<StateT, OutputT : Any>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I brought this up in the Kotlin Slack. I should file a ticket: https://kotlinlang.slack.com/archives/C0B8PUJGZ/p1555962527006200
* @see StatefulWorkflow.testRender | ||
* @see com.squareup.workflow.StatelessWorkflow.testRender | ||
*/ | ||
class MockChildWorkflow<I, R>(private val renderer: (I) -> R) : Workflow<I, Nothing, R> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FakeChildWorkflow
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It actually records the input though so you can check it later, doesn't that make it technically a "mock"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, Zach's right. Changed my vote.
https://blog.pragmatists.com/test-doubles-fakes-mocks-and-stubs-1a7491dfa3da
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, yes. This blog is handy 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although we lose symmetry with the worker then, which actually is a fake…
I want to add more instrumentation on them anyway, so they'll both be mocks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @see com.squareup.workflow.StatefulWorkflow.testRender | ||
* @see com.squareup.workflow.StatelessWorkflow.testRender | ||
*/ | ||
class MockWorker<T>(val name: String) : Worker<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FakeWorker
?
/** | ||
* A mock implementation of [Worker] for use in tests with `testRender` and [TestRenderResult]. | ||
* | ||
* Not this [Worker] can not actually emit any output itself. Use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo on Not
-> Note
(I believe this should read "Note this [Worker] can not actually emit any output...")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
* @see StatefulWorkflow.testRender | ||
* @see com.squareup.workflow.StatelessWorkflow.testRender | ||
*/ | ||
class MockChildWorkflow<I, R>(private val renderer: (I) -> R) : Workflow<I, Nothing, R> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, yes. This blog is handy 👍
…sses more directly. Given a `StatefulWorkflow` or `StatelessWorkflow`, call `.testRender()` on it. Pass the `InputT`, and for `StatefulWorkflow`s also pass the `StateT`. This returns a `TestRenderResult` which provides access to the actual rendering as well as methods to assert that children or workers were rendered and invoke their output handlers.
dc91197
to
ac5692d
Compare
ac5692d
to
0014ffb
Compare
Still LG with the tidier syntax (extension on the fakes). |
Given a
StatefulWorkflow
orStatelessWorkflow
, call.testRender()
on it.Pass the
InputT
, and forStatefulWorkflow
s also pass theStateT
.This returns a
TestRenderResult
which provides access to the actual renderingas well as methods to assert that children or workers were rendered and invoke
their output handlers.