[
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
