[
https://issues.apache.org/jira/browse/BEAM-2606?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16092812#comment-16092812
]
ASF GitHub Bot commented on BEAM-2606:
--------------------------------------
GitHub user echauchot opened a pull request:
https://github.com/apache/beam/pull/3592
[BEAM-2606] make WindowFnTestUtils use the value in addition to the
timestamp of the elements
Follow this checklist to help us incorporate your contribution quickly and
easily:
- [X] Make sure there is a [JIRA
issue](https://issues.apache.org/jira/projects/BEAM/issues/) filed for the
change (usually before you start working on it). Trivial changes like typos do
not require a JIRA issue. Your pull request should address just this issue,
without pulling in other changes.
- [X] Each commit in the pull request should have a meaningful subject
line and body.
- [X] Format the pull request title like `[BEAM-XXX] Fixes bug in
ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA
issue.
- [X] Write a pull request description that is detailed enough to
understand what the pull request does, how, and why.
- [X] Run `mvn clean verify` to make sure basic checks pass. A more
thorough check will be performed on your pull request automatically.
- [X] If this contribution is large, please file an Apache [Individual
Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
---
R: @kennknowles
Besides, I saw that the current `WindowFnTestUtils` has no test. I guess
that is because all windows use `WindowFnTestUtils` to test themselves. So I
did not add any test. That is said, the methods added in that PR are actually
called by the current windows tests but always with a `null` value in
`TimestampedValue`. I was thinking I could wait for that PR
https://github.com/apache/beam/pull/3286 to be merged. Indeed it defines a
`CustomWindowFn` in `WindowTest` that relies on values for window assignment. I
could then add a test of this `CustomWindowFn` with the new `WindowFnTestUtils`.
@kennknowles WDYT?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/echauchot/beam
use_timestampeValue_WindowFnTestUtils
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/beam/pull/3592.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #3592
----
commit 3f020043d1d261e4ec4dd19ee9ad3a82901dd83f
Author: Etienne Chauchot <[email protected]>
Date: 2017-07-10T13:57:44Z
[BEAM-2606] make WindowFnTestUtils use the value in addition to the
timestamp of the elements
----
> WindowFnTestUtils should allow using the value in addition to the timestamp
> of the elements
> -------------------------------------------------------------------------------------------
>
> Key: BEAM-2606
> URL: https://issues.apache.org/jira/browse/BEAM-2606
> Project: Beam
> Issue Type: Improvement
> Components: sdk-java-core
> Reporter: Etienne Chauchot
> Assignee: Etienne Chauchot
>
> {{WindowFnTestUtils}} relies only on timeStamps for everything related to
> windows assignment in the test helpers. But when creating a custom
> {{WindowFn}} (and most likely CustomWindow as well), that {{WindowFn}} might
> rely on element value in addition to timestamp to decide the windows that
> will be assigned to the element. To be able to test this kind of custom
> WindowFn, we need versions of the helper methods in WindowFnTestUtils that
> allow passing {{TimeStampedValues}}.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
