> On Feb. 11, 2014, 11:01 a.m., Eric Newton wrote: > > test/src/main/java/org/apache/accumulo/test/randomwalk/ConfigObjectManager.java, > > line 39 > > <https://reviews.apache.org/r/17947/diff/1/?file=482584#file482584line39> > > > > Almost every class can be considered an ObjectManager. Consider > > renaming the class ClientSideObjects or ClientObjects.
Much better names. Will do. > On Feb. 11, 2014, 11:01 a.m., Eric Newton wrote: > > test/src/main/java/org/apache/accumulo/test/randomwalk/ConfigProperties.java, > > line 23 > > <https://reviews.apache.org/r/17947/diff/1/?file=482585#file482585line23> > > > > The comment is wrong. This class doesn't create objects: it can find > > the username and password. Is it worth creating a whole class that just > > knows KEY_USERNAME and KEY_PASSWORD? The comment is left over from the refactoring process. Thanks for pointing that out, I'll fix it. I see ConfigProperties as specifically holding the unchanging properties configured through randomwalk.conf, vs. the test-specific, dynamic state information that State should hold. In a more thorough refactoring, I'd have tests use State and ConfigProperties separately. > On Feb. 11, 2014, 11:01 a.m., Eric Newton wrote: > > test/src/main/java/org/apache/accumulo/test/randomwalk/ConfigProperties.java, > > line 43 > > <https://reviews.apache.org/r/17947/diff/1/?file=482585#file482585line43> > > > > Can we use the fancy guava class to check for preconditions? Yeah, I'm not sure why I didn't just go for it. I'll switch to it. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17947/#review34188 ----------------------------------------------------------- On Feb. 11, 2014, 10:24 a.m., Bill Havanki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17947/ > ----------------------------------------------------------- > > (Updated Feb. 11, 2014, 10:24 a.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-841 > https://issues.apache.org/jira/browse/ACCUMULO-841 > > > Repository: accumulo > > > Description > ------- > > This is a refactoring of the State class used for randomwalk. I tried to > balance the extent of the refactoring with the extent of changes to the tests > themselves. So, State is still a central class, but it delegates to other > more focused classes for almost everything beyond storing just state > key/value pairs. Other notes: > > - The node visiting mechanism was unused so it's removed. Module has a > maxHops counting mechanism which does the same thing. > - The getCredentials() method was only ever used to then get an > authentication token, so the tests now get the token directly, and > getCredentials() is gone. > > > Diffs > ----- > > > test/src/main/java/org/apache/accumulo/test/randomwalk/ConfigObjectManager.java > PRE-CREATION > > test/src/main/java/org/apache/accumulo/test/randomwalk/ConfigProperties.java > PRE-CREATION > test/src/main/java/org/apache/accumulo/test/randomwalk/Module.java > 25684809daf29644550c05256c7abaf60fc2e75c > test/src/main/java/org/apache/accumulo/test/randomwalk/Node.java > 1868adedcbb41040d2b0e2e2edff642799f10673 > test/src/main/java/org/apache/accumulo/test/randomwalk/State.java > f4102ab5dbff63e18ba5673126eeb5e646b84771 > > test/src/main/java/org/apache/accumulo/test/randomwalk/multitable/CopyTable.java > 5a2172d6eddf302fb266c82e174ab235358e9cd6 > > test/src/main/java/org/apache/accumulo/test/randomwalk/security/AlterTablePerm.java > 335ae31dc05ebade9f30187a2ab4d9c79ae205c6 > > test/src/main/java/org/apache/accumulo/test/randomwalk/security/SecurityFixture.java > 30f12fbc57b625489271a3a4a23f8c21b4df9ec1 > > test/src/main/java/org/apache/accumulo/test/randomwalk/security/SetAuths.java > 54bc34aface7428c3149ffbc438cc0be82cee72f > > test/src/main/java/org/apache/accumulo/test/randomwalk/security/Validate.java > 03718ce4e044e985cb309c01c531630cf9222a33 > > test/src/main/java/org/apache/accumulo/test/randomwalk/sequential/MapRedVerify.java > f60fe44c66a6f23242aa442f279ecb86c5162109 > > test/src/main/java/org/apache/accumulo/test/randomwalk/shard/BulkInsert.java > 41acce2575de571fd48fdf5759455be3c5698e33 > > Diff: https://reviews.apache.org/r/17947/diff/ > > > Testing > ------- > > Compile works; ran MultiTable randomwalk, which does run MR jobs to test > getMapReduceJars(); also ran Security randomwalk. > > > Thanks, > > Bill Havanki > >
