[BEAM-972] Add unit tests to Gearpump runner #2302
Conversation
|
R: @kennknowles |
|
Changes Unknown when pulling eb0d333 on huafengw:addGPUT into ** on apache:gearpump-runner**. |
|
Refer to this link for build results (access rights to CI server needed): |
|
Hi Huafeng! Thanks for contributing. I am on a little vacation but I I will take a look on Monday. |
|
@kennknowles Thanks, any comment? |
There was a problem hiding this comment.
My main comment is that this PR actually does many things:
- Adds some unit tests
- Fixes up the
pom.xml - Fills in the
README.md
So that's all great :-)
Next time, if you remember to make many little commits that might be nice. Not too important right now. I'll merge this.
Other than this, I thought we set it up for Gearpump's integration tests ran on every precommit while it was on a feature branch (CC: @manuzhang). Did it take too long and time out? It doesn't look like it ran this time.
I'll merge this either way.
|
|
||
| assertNull(deserializedOptions.getEmbeddedCluster()); | ||
| assertNull(deserializedOptions.getSerializers()); | ||
| assertEquals(10, deserializedOptions.getParallelism()); |
There was a problem hiding this comment.
In Beam, our style is to use assertThat(deserializedOptions.getParallelism(), equalTo(10)).
|
Thanks Kenn, I'll keep your advice in mind |
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify. (Even better, enableTravis-CI on your fork and ensure the whole test matrix passes).
<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.