----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18625/#review35786 -----------------------------------------------------------
Generally, would recommend that most of these setup (and testing) functions be broken into their own scripts for readability. You're already passing numerous variables to each function, it'd be just as simple (and much more readable) to pass them to different external scripts. test/system/upgrade_test.sh <https://reviews.apache.org/r/18625/#comment66528> +1 test/system/upgrade_test.sh <https://reviews.apache.org/r/18625/#comment66529> If you're going to test for the existence of TEMP_DIR dir, use that variable when creating the directory. test/system/upgrade_test.sh <https://reviews.apache.org/r/18625/#comment66530> Recommend grep arguments before search argument for readability. grep -m 1 <search> <files> Alternative/preferably, since you don't need the output of the POPULATED string, you could catch the exit code of grep using $? and test on that. test/system/upgrade_test.sh <https://reviews.apache.org/r/18625/#comment66531> Script is performing a LOT of setup and I/O within the temp directory, would strongly recommend passing the desired working directory as an argument; easier to re-use script to do multiple test runs with different configurations. - Alex Moundalexis On Feb. 28, 2014, 2:23 p.m., John McNamee wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18625/ > ----------------------------------------------------------- > > (Updated Feb. 28, 2014, 2:23 p.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-2145 > https://issues.apache.org/jira/browse/ACCUMULO-2145 > > > Repository: accumulo > > > Description > ------- > > This is still a work in progress. > > The framework would configure the Accumulo versions, HDFS, zookeeper, and > which test to run. > Runs a set of upgrade tests. > > > Diffs > ----- > > test/system/upgrade_test.sh 6259e1c > > Diff: https://reviews.apache.org/r/18625/diff/ > > > Testing > ------- > > > Thanks, > > John McNamee > >
