> 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. > > kturner wrote: > 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.
Trying to wrap your head around even how the RW Framework gets instantiated, how Fixtures, Tests and Modules are used, much less the lifetime of State makes it hard to even understand how the tests run. Any changes to RW that can help make things more obvious would be awesome. - Josh ----------------------------------------------------------- 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 > >
