accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Havanki" <bhava...@clouderagovt.com>
Subject Re: Review Request 15943: ACCUMULO-1556 - Clarify initialization error for used fs [master]
Date Mon, 02 Dec 2013 19:06:05 GMT


> On Dec. 2, 2013, 10:53 a.m., Josh Elser wrote:
> > Adding powermock as well isn't the end of the world (especially since it would be
in the test scope). It's up to you :shrug:

Mocking a static method is often considered a workaround for what should be better designed
for testability. I'd personally rather leave it out, and revisit it later with many other
cases like it.

Thanks for the review!


- Bill


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15943/#review29582
-----------------------------------------------------------


On Dec. 2, 2013, 10:41 a.m., Bill Havanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15943/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2013, 10:41 a.m.)
> 
> 
> Review request for accumulo and John Vines.
> 
> 
> Bugs: ACCUMULO-1556
>     https://issues.apache.org/jira/browse/ACCUMULO-1556
> 
> 
> Repository: accumulo
> 
> 
> Description
> -------
> 
> Patch for ACCUMULO-1556 for the master branch, as requested by vines. I post this review
because of the shift from FileSystem to VolumeManager, so having extra eyes on it is good
IMO.
> 
> 
> Diffs
> -----
> 
>   server/base/pom.xml c2054bbe3bd7b7db3aa2e209e2a67b9c1c2dc1ad 
>   server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java 0477a443e307e174965140f0fd14c898a0d98727

>   server/base/src/test/java/org/apache/accumulo/server/init/InitializeTest.java PRE-CREATION

> 
> Diff: https://reviews.apache.org/r/15943/diff/
> 
> 
> Testing
> -------
> 
> Unit test added and passed. I had to comment out a test from the earlier branches due
to being unable to mock a static method on FileSystem (and I didn't want to introduce Powermock
just for that).
> 
> 
> Thanks,
> 
> Bill Havanki
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message