mrunit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dave Beech (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (MRUNIT-119) Counter tests should support assertions that counters are not present
Date Mon, 25 Jun 2012 07:28:42 GMT

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

Dave Beech commented on MRUNIT-119:
-----------------------------------

Hi Bertrand

Looks good so far. I haven't had time to check it properly yet unfortunately, but here are
a couple of things I noticed:

- The patch is in git format - could you provide one against the current trunk of the svn
repo? I'm not sure our git mirror is completely up-to-date.
- Please use 2 spaces for indenting rather than 4 spaces or a tab (2 spaces is the Hadoop
standard)
- Could you please add unit tests to cover the use of mapreduce api as well as mapred, and
enum counters as well as the string pairs. I realise the code is the same for each right now,
but because of the duplication we may introduce bugs if we change one api and forget to update
the other. 

Also, the use case of the new feature is different to how I originally interpreted your message
on the mailing list, so I'd like to double check that I understand your requirement correctly.
Is the following right? You have a counter, for example an error counter, that you want to
ensure does not get set. However, the counter name can be dynamic so you cannot know up-front
what to assert in the unit test. 

I can see how the "strict mode" would solve this, as you're basically saying "I don't know
what the error will look like, so fail the test if you see anything other than those counters
I was expecting". However, if you have dynamic "non-error" counters too, or counters you simply
don't care about either way, the strict mode wouldn't be suitable. I am concerned that the
strict mode might make the unit tests unnecessarily brittle, as you will no doubt add / remove
/ rename valid counters during development and refactoring, and this will make all your strict
counter tests fail. This might be OK, but I can imagine it being inconvenient for some. 

How I originally imagined the feature when I read the message was to add something like driver.withNoCounter("Error",
"Some error") or even a regex version driver.withNoCounter("Error", ".+Exception") - so you
can selectively assert that certain counters should not be present, but ignore everything
else. 

Jarek - as you added the counter checking code originally, what do you think? Does anyone
else have any thoughts on this?



                
> Counter tests should support assertions that counters are not present
> ---------------------------------------------------------------------
>
>                 Key: MRUNIT-119
>                 URL: https://issues.apache.org/jira/browse/MRUNIT-119
>             Project: MRUnit
>          Issue Type: Improvement
>    Affects Versions: 1.0.0
>            Reporter: Dave Beech
>            Priority: Minor
>         Attachments: mrunit-119-proposal.diff
>
>
> From Bertrand Dechoux via user mailing list.
> Should we add a feature to the counter checking feature so that users can assert that
something will *not* be present, for example an error or exception counter, and have the test
fail if it is set?

--
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

        

Mime
View raw message