Skip to content
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

Make verifyAction and verifyActionResult support no processed action. #909

Merged
merged 1 commit into from Jan 24, 2020

Conversation

zach-klippenstein
Copy link
Collaborator

@zach-klippenstein zach-klippenstein commented Jan 24, 2020

Closes #885.

cc @ijwhelan

@zach-klippenstein zach-klippenstein added enhancement New feature or request testing labels Jan 24, 2020
@zach-klippenstein zach-klippenstein added this to the kotlin v0.23.0 milestone Jan 24, 2020
@zach-klippenstein zach-klippenstein added this to Needs review in Workflow (Kotlin) via automation Jan 24, 2020
/**
* Asserts that the render pass either didn't result in any [WorkflowAction] being performed
* (e.g. in the case where a rendering event is disabled by being mapped to a no-op), or that
* an action was performed that didn't change the state or emit an output.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is this too lenient? Should this method require no action to have been processed at all? The current behavior could let through an action that has complex logic that just happens to set the state to it's previous value.

Copy link
Contributor

Choose a reason for hiding this comment

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

My knee-jerk is that a literal "no action was fired" assertion is too much of an implementation detail to encourage in testing. But the real question is whether this satisfies the original requester.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

An alternative is to allow verifyActionResult to handle the case where no action was fired and just treat it like a no-op action, but that raises the same concern about actions that "just happen" to be no-op.

Choose a reason for hiding this comment

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

So the original use case I had in mind for this was verifying the following behavior:

  • User has entered invalid data into a simple form
  • User presses the submit button
  • Ensure that no network request was made to actually submit the data

The motivation for this test was to ensure that I'm being properly defensive against the UI not respecting the data validity flag in the state and leaving the submit button enabled. I want to make sure that the lambda I'm handing to the UI to fire when the submit button is clicked is actually a no-op.

I agree that actions are implementation details and I don't want to expose them outside of the workflow, but I think allowing no-op actions might open up some gaps in a test like the one I'm trying to write.

@zach-klippenstein
Copy link
Collaborator Author

Reworked this PR: no more verifyNoAction, instead verifyAction and verifyActionResult let you assert noAction.

@zach-klippenstein zach-klippenstein changed the title Introduce RenderTestResult.verifyNoAction. Make verifyAction and verifyActionResult support no processed action. Jan 24, 2020
@rjrjr
Copy link
Contributor

rjrjr commented Jan 24, 2020

Still LG.

Copy link
Contributor

@dhavalshreyas dhavalshreyas left a comment

Choose a reason for hiding this comment

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

Sweet!

@zach-klippenstein zach-klippenstein merged commit 6f5809a into master Jan 24, 2020
Workflow (Kotlin) automation moved this from Needs review to Done Jan 24, 2020
@zach-klippenstein zach-klippenstein deleted the zachklipp/rendertester-noaction branch January 24, 2020 21:38
@zach-klippenstein zach-klippenstein added the kotlin Affects the Kotlin library. label Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request kotlin Affects the Kotlin library. testing
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

renderTester API should support verifying that render event did not send to sink
4 participants