> On Feb. 11, 2014, 4:01 p.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. > > Bill Havanki wrote: > Much better names. Will do.
Naming stuff sucks. I was thinking about what the goal of this refactoring is. One reason is to make it easier for someone to write random walk test. Looking at the existing code I think one possible source of confusion is that methods intended for use by the framework itself are public. Someone trying to write RW test has to spend time figuring this out so they can ignore those methods. Maybe rename ClientObjectManager to TestEnvironment (I think this communicates its purpose better) and add the username and password methods to it. Looking at the existing getProperties method on State, its only used by the framework code and holds config for the framework. Could possibly make getProperties() package private and add javadoc about it returning framework config. There are some other things in state that are not used by test and could be made package private. - kturner ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17947/#review34188 ----------------------------------------------------------- On Feb. 11, 2014, 3:24 p.m., Bill Havanki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/17947/ > ----------------------------------------------------------- > > (Updated Feb. 11, 2014, 3:24 p.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 > >
