Make unit testing possible by switching to base context#17
Make unit testing possible by switching to base context#17richardjharding wants to merge 1 commit intomikhailshilkov:masterfrom
Conversation
mikhailshilkov
left a comment
mikhailshilkov
left a comment
There was a problem hiding this comment.
Left a couple of comments and questions but overall LGTM. Thanks so much for exploring the testing aspect and making the PR!
| @@ -0,0 +1,45 @@ | |||
| module samples.unittest | |||
There was a problem hiding this comment.
Nit: the file name and module name should be in PascalCase to match other samples
|
|
||
| open Microsoft.Azure.WebJobs | ||
| open DurableFunctions.FSharp | ||
| open System |
|
|
||
| let workflowWithParam delay = orchestrator { | ||
| let! s = Activity.call printTime DateTime.Now | ||
| let! (s:string) = Activity.call printTime DateTime.Now |
There was a problem hiding this comment.
This is really weird indeed. I can't see why type inference is broken... Almost seems like a bug in the compiler. Maybe we should change the example not to use .Contains to avoid that.
|
|
||
| // returns ["Hello Tokyo!", "Hello Seattle!", "Hello London!"] | ||
| return [hello1; hello2; hello3] | ||
| } |
There was a problem hiding this comment.
Consider using a strongly typed implementation style. I don't want to give the impression that only callByName style is testable. (I hope others are, too)
| let mockContext = Mock<DurableOrchestrationContextBase>() | ||
| mockContext.Setup(fun c -> c.CallActivityAsync<string>("SayHello","Tokyo")).ReturnsAsync("Hello Tokyo") |> ignore | ||
| mockContext.Setup(fun c -> c.CallActivityAsync<string>("SayHello","Seattle")).ReturnsAsync("Hello Seattle") |> ignore | ||
| mockContext.Setup(fun c -> c.CallActivityAsync<string>("SayHello","London")).ReturnsAsync("Hello London") |> ignore |
There was a problem hiding this comment.
This looks quite mechanic... Can you think of any helper functions that we could provide to make it read smoother? Aren't there any F#-y ways to mock things?
There was a problem hiding this comment.
yeah I wanted to use the Foq library to mock as it has a much nicer way of setting up the expectations using quotations but it doesn't work with the DurableOrchestrationContextBase - one of the properties has a sealed setter that causes Foq to fail (someone else already raised that issue so if it gets fixed I can swap it in)
| sprintf "Hello %s!" name | ||
|
|
||
| [<FunctionName("HelloSequenceTest")>] | ||
| let RunWorkflow ([<OrchestrationTrigger>] context: DurableOrchestrationContextBase) = |
There was a problem hiding this comment.
What would you think of the following style of the test:
- Define an activity (as in
Typedexample) - Define an orchestrator value. Don't use the activity directly but accept it as a parameter.
- Workflow Function calls the orchestrator with the real activity as the argument
- The test calls the orchestrator with a dummy activity as the argument, which returns hard-coded values. No mocks needed.
Could you try this out and let me know your opinion?
There was a problem hiding this comment.
Yep - makes sense - I'll try that and see how it goes
2 participants
#16 First attempt - switch to using
DurableOrchestrationContextBasealso added a sample Expecto/ Moq unit testHad to make a couple of changes to the existing samples which means this is probably a breaking change
In
Eternal.fsI had to add a type annotation to the result of the activity call - not entirely sure why that was required