VcfToBqTask unit tests#19
Conversation
| gcp-variant-transforms-java.iml | ||
|
|
||
| #Ignore VSCode files | ||
| .vscode/ |
There was a problem hiding this comment.
Wasn't this added in previous PRs? Doesn't matter much, but maybe some commit mishap.
There was a problem hiding this comment.
I'm not sure if this commit still affects this PR. Maybe we can take it out
| bind(VcfParser.class).to(VcfParserImpl.class); | ||
| bind(Task.class).to(VcfToBqTask.class); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
This should had been created in Zhiqiang's PR right? Could you sync and update the PR?
| VcfToBqTask vcfToBqTask; | ||
| VcfToBqTask spyVcfToBqTask; | ||
|
|
||
| @Mock |
There was a problem hiding this comment.
I think we can inline, just to save space stylistically:
@mock PipelineRunner mockedPipelineRunner.
Here and below.
| ImmutableList<String> headerLines = headerLinesBuilder.build(); | ||
| doReturn(headerLines).when(mockedHeaderReader).getHeaderLines(); | ||
| doNothing().when(spyVcfToBqTask).setPipelineOptions(argCaptorPipelineOptions.capture()); | ||
| doNothing().when(mockedVcfToBqContext).setHeaderLines(argCaptorImmutableList.capture()); |
There was a problem hiding this comment.
Can't we, specifically for this case, use headerLines instead of capturing? The problem with PipelineOptions was that the object was changed (type casted into PipelineOptions). Here I don't think we have the same issue.
| setUpRunBehaviors(); | ||
| spyVcfToBqTask.run(); | ||
|
|
||
| verify(mockedVcfToBqContext).setHeaderLines(argCaptorImmutableList.capture()); |
There was a problem hiding this comment.
I think this part we can just check by checking that it was called with headerLines (since they have not been modified).
|
|
||
| @Test | ||
| public void testVcfTask_whenSetPipelineOptions_thenVerify() throws Exception { | ||
| doNothing().when(spyVcfToBqTask).setPipelineOptions(argCaptorPipelineOptions.capture());; |
There was a problem hiding this comment.
This test is kinda pointless, tbh. We just call the function here and verify that function has been called. I say we remove it.
| } | ||
|
|
||
| @Test | ||
| public void testVcfTask_whenRun_thenVerifySetPipelineOptions() throws IOException { |
There was a problem hiding this comment.
I think we can combine all three tests into one.
| setUpRunBehaviors(); | ||
| spyVcfToBqTask.run(); | ||
|
|
||
| verify(spyVcfToBqTask).setPipelineOptions(mockedVcfToBqOptions); |
There was a problem hiding this comment.
I'm surprised this passes... wasn't the whole purpose of using arg captor was that the value was modified on runtime and thus, we couldn't have expected it?
2 participants
Unit tests for VcfToBqTask.java: