[jira] [Issue Comment Edited] (MRUNIT-68) Support custom counter checking

["Jarek Jarcec Cecho (Issue Comment Edited) (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/MRUNIT-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231888#comment-13231888 ] 

Jarek Jarcec Cecho edited comment on MRUNIT-68 at 3/17/12 8:04 AM:
-------------------------------------------------------------------

HI Jim and Brock,
I've updated my patch with your suggestions, however not all of them were met. Please see my comments inline (sorry for the formatting, I'm not sure why Apache instance of JIRA is ignoring formatting tags):

bq. CounterWrapper should use private instance variables 

I've changed the type to explicit private.

bq. CounterWrapper can use else statements in the findCounterValue methods, since if one counters variable is null the other must not be 

Changed. I do agree that only one counter might be filled. My original intention with two if statements was to have defined default value that can be used for distinguishing case where the counter is not present at all.

bq. TestDriver's with methods dont need to return this, they should return void since the calling methods wont use that result 

Not changed. I do agree that return value in TestDriver.withCounter() is not used, however changing it to void will result in compile errors:

{code}
[ERROR] /home/jarcec/projects/apache/mrunit/src/main/java/org/apache/hadoop/mrunit/MapReduceDriver.java:[381,49] withCounter(java.lang.String,java.lang.String,long) in org.apache.hadoop.mrunit.MapReduceDriver cannot override withCounter(java.lang.String,java.lang.String,long) in org.apache.hadoop.mrunit.TestDriver; attempting to use incompatible return type
[ERROR] found   : org.apache.hadoop.mrunit.MapReduceDriver<K1,V1,K2,V2,K3,V3>
[ERROR] required: void
{code}

bq. TestDriver's validate should use fail(msg) not RuntimeException 

Changed. Thank you for pointing this out. I've started working on the patch when we were still using RuntimeExceptions.

bq. You should use the ExpectedSuppliedException class in test mrunit to verify the exception message returned 

Changed as well.

bq. I think 0 would be a clearer default value

I've removed default value completely.
                
      was (Author: jarcec):
    HI Jim and Brock,
I've updated my patch with your suggestions, however not all of them were met. Please see my comments inline:

bq. CounterWrapper should use private instance variables 

I've changed the type to explicit private.

bq. CounterWrapper can use else statements in the findCounterValue methods, since if one counters variable is null the other must not be 

Changed. I do agree that only one counter might be filled. My original intention with two if statements was to have defined default value that can be used for distinguishing case where the counter is not present at all.

bq. TestDriver's with methods dont need to return this, they should return void since the calling methods wont use that result 

Not changed. I do agree that return value in TestDriver.withCounter() is not used, however changing it to void will result in compile errors:

{code}
[ERROR] /home/jarcec/projects/apache/mrunit/src/main/java/org/apache/hadoop/mrunit/MapReduceDriver.java:[381,49] withCounter(java.lang.String,java.lang.String,long) in org.apache.hadoop.mrunit.MapReduceDriver cannot override withCounter(java.lang.String,java.lang.String,long) in org.apache.hadoop.mrunit.TestDriver; attempting to use incompatible return type
[ERROR] found   : org.apache.hadoop.mrunit.MapReduceDriver<K1,V1,K2,V2,K3,V3>
[ERROR] required: void
{code}

bq. TestDriver's validate should use fail(msg) not RuntimeException 

Changed. Thank you for pointing this out. I've started working on the patch when we were still using RuntimeExceptions.

bq. You should use the ExpectedSuppliedException class in test mrunit to verify the exception message returned 

Changed as well.

bq. I think 0 would be a clearer default value

I've removed default value completely.
                  
> Support custom counter checking
> -------------------------------
>
>                 Key: MRUNIT-68
>                 URL: https://issues.apache.org/jira/browse/MRUNIT-68
>             Project: MRUnit
>          Issue Type: New Feature
>    Affects Versions: 0.8.1
>            Reporter: Jarek Jarcec Cecho
>             Fix For: 1.0.0
>
>         Attachments: MRUNIT-68.patch, MRUNIT-68.patch, MRUNIT-68.patch, MRUNIT-68.patch
>
>
> It would be great if user could check custom counter values in addition to checking outputs. Let me show my idea on example, right now counters needs to be checked explicitly:
> assertEquals(2, mapDriver.getCounters().findCounter(CustomMapper.CustomCounter.NAME).getValue());
> It would be great if user could do something like:
> .withCounter(CustomMapper.CustomCounter.Name, 2)

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira