[
https://issues.apache.org/jira/browse/MAPREDUCE-1569?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12844196#action_12844196
]
Aaron Kimball commented on MAPREDUCE-1569:
------------------------------------------
Hi Chris,
This looks like a good start! The test failure in Hudson is unrelated to this
patch.
Here's some comments on the patch itself:
* Can you wrap your new lines at 80 columns?
* The withConfiguration() methods will need to be pushed down (as they are)
into all the specific classes (MapDriver, ReduceDriver, etc). But I think the
getConfiguration() method can live in TestDriver, eliminating redundant code.
* For API consistency, we should have a {{public void
setConfiguration(Configuration conf)}} method of TestDriver as well. (And
{{withConfiguration()}} should call {{setConfiguration()}} rather than
manipulating the field explicitly.)
* withConfiguration()'s Javadoc {...@return}} needs to be filled out.
* For MockMapContext/MockReduceContext, instead of changing the existing
constructor, can you add another constructor that takes the configuration as an
argument? The existing constructor can call the new one with a {{new
Configuration()}} there.
When you fix these, please just attach another patch to this issue and redo the
cancel patch / submit patch dance.
> Mock Contexts & Configurations
> ------------------------------
>
> Key: MAPREDUCE-1569
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-1569
> Project: Hadoop Map/Reduce
> Issue Type: Improvement
> Components: contrib/mrunit
> Reporter: Chris White
> Assignee: Chris White
> Priority: Minor
> Attachments: MAPREDUCE-1569.patch, MAPREDUCE-1569.patch
>
> Original Estimate: 1h
> Remaining Estimate: 1h
>
> Currently the library creates a new Configuration object in the
> MockMapContext and MocKReduceContext constructors, rather than allowing the
> developer to configure and pass their own
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
