+1 -- sounds like a useful addition!

On Wed, May 10, 2017 at 8:27 AM Dan Halperin <[email protected]> wrote:

> Hey Michael,
>
> That TestRule sounds like it might be pretty useful generally; would it be
> worth contributing back to Beam?
>
> On Wed, May 10, 2017 at 4:12 AM, Michael Luckey <[email protected]>
> wrote:
>
>> Hi Pablo,
>>
>> we ended up to follow the path with setup of the global environment.
>>
>> Therefore we implemented a simple junit TestRule, which is used something
>> along the following:
>>
>>     @Rule TestMetrics metrics = new TestMetrics();
>>
>>      ....
>>
>>     @Test
>>      public void invalids()  {
>>          final DoFnTester<InputT, OutputT> doFnTester =
>> DoFnTester.of(fixture);
>>          doFnTester.processElement(input);
>>
>>          assertThat(metrics,counterValue(fixture.ctr), is(1L));
>>      }
>>
>> Thats pretty close to what we did before and seems to be working very
>> well.
>>
>> So again, thanks a lot for your help, very much appreciated,
>>
>> michel
>>
>> On Wed, May 10, 2017 at 11:01 AM, Michael Luckey <[email protected]>
>> wrote:
>>
>>> Hi Pablo,
>>>
>>> thx for the pointers. I'll have a look into it and let you know.
>>>
>>> Regards,
>>>
>>> michel
>>>
>>> On Wed, May 10, 2017 at 3:18 AM, Pablo Estrada <[email protected]>
>>> wrote:
>>>
>>>> Hi Michael,
>>>> I'm sorry. I see I did not read your first email properly.
>>>>
>>>> There are a couple places in the core SDK or runner code and tests that
>>>> used to use aggregators, and now use metrics. There are two reasonable
>>>> options for this:
>>>> 1. In [1], the test sets up the metrics global environment by setting
>>>> the current container (e.g.  MetricsEnvironment.setCurrentContainer(new
>>>> MetricsContainer("anystep"));), and the LateDataFilter uses metrics
>>>> normally[2], by creating a counter that relies on the environment set up in
>>>> the test.
>>>>
>>>> 2. If you'd rather not rely on setting up a global environment, you can
>>>> use CounterCell, and pass it in to your test. In [3] it's not a test, but a
>>>> CounterCell is still created to keep internal statistics, and later its
>>>> value is checked [4]. As a note, you may note in [3] that CounterCells are
>>>> a bit quirky to create, as we did not intend for external users to be able
>>>> to create them.
>>>>
>>>> Let me know if these suggestions are helpful.
>>>> Best
>>>> -P.
>>>>
>>>> [1] -
>>>> https://github.com/apache/beam/blob/master/runners/core-java/src/test/java/org/apache/beam/runners/core/LateDataDroppingDoFnRunnerTest.java#L61
>>>>
>>>> [2] -
>>>> https://github.com/apache/beam/blob/master/runners/core-java/src/main/java/org/apache/beam/runners/core/LateDataDroppingDoFnRunner.java#L96
>>>> [3] -
>>>> https://github.com/apache/beam/blob/master/runners/spark/src/main/java/org/apache/beam/runners/spark/stateful/SparkGroupAlsoByWindowViaWindowSet.java#L210
>>>> [4] -
>>>> https://github.com/apache/beam/blob/master/runners/spark/src/main/java/org/apache/beam/runners/spark/stateful/SparkGroupAlsoByWindowViaWindowSet.java#L326
>>>>
>>>> On Tue, May 9, 2017 at 4:33 PM Michael Luckey <[email protected]>
>>>> wrote:
>>>>
>>>>> Hi Pablo,
>>>>>
>>>>> thanks for your help! We certainly could change our testing code and
>>>>> involve execution of a pipeline during tests.
>>>>>
>>>>> But currently we are leveraging DoFnTester, i.e. scoping our tests to
>>>>> the DoFn only, which means, there is neither a pipeline nor a pipeline
>>>>> result involved, which i could call upon.
>>>>>
>>>>> It might be a bad idea trying to test counters on this basis, but as
>>>>> it was supported previously i thought we might have overlooked an API for
>>>>> accessing these metrics somehow within DoFnTesting. Not sure, wether it
>>>>> makes sense for the DoFnTester to somehow provide Metrics-Support to 
>>>>> enable
>>>>> this kind of tests. I certainly do not like the idea to much starting to 
>>>>> do
>>>>> some mocking of the metrics api within my test implementation.
>>>>>
>>>>> Regards,
>>>>>
>>>>> michel
>>>>>
>>>>>
>>>>> On Wed, May 10, 2017 at 1:10 AM, Pablo Estrada <[email protected]>
>>>>> wrote:
>>>>>
>>>>>> Hi Michael,
>>>>>> For the Metrics API, the way to programatically query the value of a
>>>>>> metric is by using the MetricResults.queryMetrics method. You get the
>>>>>> MetricResults object from the PipelineResult object, and query it like 
>>>>>> so:
>>>>>>
>>>>>> PipelineResult res = p.run();
>>>>>> MetricQueryResults metricResult = res.metrics().queryMetrics(....);
>>>>>>
>>>>>> The queryMetrics method takes in a MetricsFilter instance.
>>>>>>
>>>>>> Not all runners support this operation. For the dataflow runner, PR
>>>>>> 2896[1] should add it.
>>>>>>
>>>>>> Let me know if you need more help with this.
>>>>>> Best
>>>>>> -P.
>>>>>>
>>>>>> [1] - https://github.com/apache/beam/pull/2896
>>>>>>
>>>>>> On Tue, May 9, 2017 at 3:48 PM Michael Luckey <[email protected]>
>>>>>> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> currently we are evaluating a migration from 0.6.0 to current. We
>>>>>>> encountered the following problem, which we currently not sure, how to 
>>>>>>> best
>>>>>>> solve.
>>>>>>>
>>>>>>> Say we have a DoFn, which is using Aggregators, e.g.
>>>>>>>
>>>>>>>         ctr = createAggregator("someCounter", Sum.ofLongs());
>>>>>>>
>>>>>>>  We were testing them with DoFn-Tester like
>>>>>>>
>>>>>>>         final DoFnTester<Record, Record> doFnTester =
>>>>>>> DoFnTester.of(fixture);
>>>>>>>         doFnTester.processElement(input);
>>>>>>>
>>>>>>>
>>>>>>>  assertThat(doFnTester.getAggregatorValue(fixture.ctr).longValue(), 
>>>>>>> is(1L));
>>>>>>>
>>>>>>> As aggregators are removed now from the codebase, we are considering
>>>>>>> using Metrics instead. But we did not find an equivalent to the
>>>>>>> getAggregatorValue method on DoFnTester.
>>>>>>>
>>>>>>> Any suggestion how we could keep our counters tested within a unit
>>>>>>> test based on DoFnTester? Or do we have to find a completely different
>>>>>>> solution? Are we doing something completely wrong trying to test correct
>>>>>>> workings of our counters with this approach?
>>>>>>>
>>>>>>> Regards,
>>>>>>>
>>>>>>> michel
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>
>>
>

Reply via email to