Skip to content

Fixing RenderTester assert(action:) #57

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

Merged
merged 1 commit into from
Aug 25, 2020

Conversation

dhavalshreyas
Copy link
Contributor

In AnyWorkflowConvertible we were mapping all actions to AnyWorkflowAction. Fixing it here to map to a generic Action type.

Checklist

  • Unit Tests
  • I have made corresponding changes to the documentation

Copy link
Collaborator

@bencochran bencochran left a comment

Choose a reason for hiding this comment

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

This ended up being a lot less invasive than I had imagined, nice!

@@ -38,7 +38,7 @@ extension AnyWorkflowConvertible {
///
/// - Returns: The `Rendering` generated by the workflow.
public func rendered<Parent>(in context: RenderContext<Parent>, key: String = "") -> Rendering where Output: WorkflowAction, Output.WorkflowType == Parent {
return asAnyWorkflow().render(context: context, key: key, outputMap: { AnyWorkflowAction($0) })
return asAnyWorkflow().render(context: context, key: key, outputMap: { $0 })
}

public func rendered<Parent, Action>(in context: RenderContext<Parent>, key: String = "", outputMap: @escaping (Output) -> Action) -> Rendering where Action: WorkflowAction, Action.WorkflowType == Parent {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The line below this too, yeah? (github won’t let me comment on collapsed parts of code…)

@@ -98,6 +98,19 @@ final class WorkflowRenderTesterTests: XCTestCase {
.assertNoAction()
}

func test_childWorkflowAction() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would this test fail before? I thought this only happened when things got AnyWorkflowd?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test did fail before the fix.

@dhavalshreyas dhavalshreyas force-pushed the dhaval/workflowAssertActionFix branch from bd9e188 to 542fd03 Compare August 25, 2020 22:33
@dhavalshreyas dhavalshreyas merged commit 0b65bcf into main Aug 25, 2020
@dhavalshreyas dhavalshreyas deleted the dhaval/workflowAssertActionFix branch August 25, 2020 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants