> On April 15, 2014, 2:11 p.m., Mike Drob wrote: > > This seems inelegant, but I'm not sure there is a better way. Maybe we > > could provide a class level annotation and then extract values out of there? > > > > Might make sense to make the defaultTimeoutMillis method abstract, to force > > everybody to override it. I can't think of a time when a class should have > > no timeout and this might lull somebody into forgetting to add one.
A class level annotation would be more elegant, but at this point I don't think it's substantively different. I'm not in a favor of forcing a defaultTimeout, because I had to move from defaulting to 60 seconds (which is what I'd prefer if we're going to say there should always be a timeout) due to the fact that it's a ceiling. For tests where there are a bunch of different method-level annotations, it's just noise. Also, personally I don't think any of the tests should have JUnit timeouts. JUnit's ability to handle timing restrictions are very limited (as evidenced by this very hacky way we have to go about parameterizing things). I plan to always run these with the factor set to 0 so that I can allow things with better information about the environment and history to handle timeouts, namely Jenkins. Given that, it seems silly to turn around and tell the developer of a new IT that they need to know how long a given method will take. - Sean ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20331/#review40391 ----------------------------------------------------------- On April 15, 2014, 9:01 a.m., Sean Busbey wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20331/ > ----------------------------------------------------------- > > (Updated April 15, 2014, 9:01 a.m.) > > > Review request for accumulo. > > > Bugs: ACCUMULO-2666 > https://issues.apache.org/jira/browse/ACCUMULO-2666 > > > Repository: accumulo > > > Description > ------- > > Creates a default timeout for all functional ITs in a way that we can scale > at test time via a system property, using a JUnit Rule instead of the timeout > parameter to the Test annotation. > > per-method annotation can still override this (and still does in a few cases) > > > Diffs > ----- > > test/pom.xml 4ec6f6a > test/src/test/java/org/apache/accumulo/test/functional/AbstractMacIT.java > 352470c > test/src/test/java/org/apache/accumulo/test/functional/AddSplitIT.java > cc2285e > test/src/test/java/org/apache/accumulo/test/functional/BackupMasterIT.java > 7c1f8a2 > > test/src/test/java/org/apache/accumulo/test/functional/BadIteratorMincIT.java > a25e775 > > test/src/test/java/org/apache/accumulo/test/functional/BalanceAfterCommsFailureIT.java > a16ec2f > > test/src/test/java/org/apache/accumulo/test/functional/BatchScanSplitIT.java > 22f0d98 > > test/src/test/java/org/apache/accumulo/test/functional/BatchWriterFlushIT.java > 34fb402 > test/src/test/java/org/apache/accumulo/test/functional/BigRootTabletIT.java > 0e0671b > test/src/test/java/org/apache/accumulo/test/functional/BinaryIT.java > d5a4ffd > test/src/test/java/org/apache/accumulo/test/functional/BinaryStressIT.java > 7338095 > test/src/test/java/org/apache/accumulo/test/functional/BloomFilterIT.java > 9ba713d > test/src/test/java/org/apache/accumulo/test/functional/BulkFileIT.java > 10fb7f4 > test/src/test/java/org/apache/accumulo/test/functional/BulkIT.java faa9391 > > test/src/test/java/org/apache/accumulo/test/functional/BulkSplitOptimizationIT.java > f9abc0d > > test/src/test/java/org/apache/accumulo/test/functional/ChaoticBalancerIT.java > 67a2d8c > test/src/test/java/org/apache/accumulo/test/functional/ClassLoaderIT.java > adc49d9 > test/src/test/java/org/apache/accumulo/test/functional/CleanTmpIT.java > 3ad9a3c > test/src/test/java/org/apache/accumulo/test/functional/CleanUpIT.java > 2c878e3 > test/src/test/java/org/apache/accumulo/test/functional/CloneTestIT.java > 29f838b > test/src/test/java/org/apache/accumulo/test/functional/CombinerIT.java > be1a709 > test/src/test/java/org/apache/accumulo/test/functional/CompactionIT.java > e7ccdd2 > test/src/test/java/org/apache/accumulo/test/functional/ConcurrencyIT.java > b2d16ad > test/src/test/java/org/apache/accumulo/test/functional/ConstraintIT.java > ef2212d > test/src/test/java/org/apache/accumulo/test/functional/CreateAndUseIT.java > 3dbf5ce > > test/src/test/java/org/apache/accumulo/test/functional/CreateManyScannersIT.java > e627218 > > test/src/test/java/org/apache/accumulo/test/functional/DeleteEverythingIT.java > e251157 > test/src/test/java/org/apache/accumulo/test/functional/DeleteIT.java > fe51039 > test/src/test/java/org/apache/accumulo/test/functional/DeleteRowsIT.java > 0731e44 > > test/src/test/java/org/apache/accumulo/test/functional/DeleteRowsSplitIT.java > 0a0b0b9 > > test/src/test/java/org/apache/accumulo/test/functional/DeleteTableDuringSplitIT.java > cd69be7 > > test/src/test/java/org/apache/accumulo/test/functional/DynamicThreadPoolsIT.java > c89b8ce > > test/src/test/java/org/apache/accumulo/test/functional/FateStarvationIT.java > 6ac2ef9 > > test/src/test/java/org/apache/accumulo/test/functional/HalfDeadTServerIT.java > 0346f2f > test/src/test/java/org/apache/accumulo/test/functional/LargeRowIT.java > 31783c4 > > test/src/test/java/org/apache/accumulo/test/functional/LateLastContactIT.java > fc2ed52 > test/src/test/java/org/apache/accumulo/test/functional/LogicalTimeIT.java > add7d8a > test/src/test/java/org/apache/accumulo/test/functional/MapReduceIT.java > ee5831b > > test/src/test/java/org/apache/accumulo/test/functional/MasterAssignmentIT.java > 354a97d > > test/src/test/java/org/apache/accumulo/test/functional/MasterFailoverIT.java > 8fd1499 > test/src/test/java/org/apache/accumulo/test/functional/MaxOpenIT.java > 72ad0f7 > > test/src/test/java/org/apache/accumulo/test/functional/MetadataMaxFiles.java > b83a7de > test/src/test/java/org/apache/accumulo/test/functional/MetadataSplitIT.java > 3339698 > test/src/test/java/org/apache/accumulo/test/functional/ReadWriteIT.java > e845d99 > test/src/test/java/org/apache/accumulo/test/functional/RenameIT.java > 135b4e0 > test/src/test/java/org/apache/accumulo/test/functional/RestartStressIT.java > 06cdb8c > test/src/test/java/org/apache/accumulo/test/functional/RowDeleteIT.java > 886af49 > test/src/test/java/org/apache/accumulo/test/functional/ScanIteratorIT.java > c62592b > test/src/test/java/org/apache/accumulo/test/functional/ScanRangeIT.java > 818cf92 > > test/src/test/java/org/apache/accumulo/test/functional/ScanSessionTimeOutIT.java > 693a67d > test/src/test/java/org/apache/accumulo/test/functional/ScannerIT.java > 3fca421 > > test/src/test/java/org/apache/accumulo/test/functional/ServerSideErrorIT.java > e68514d > test/src/test/java/org/apache/accumulo/test/functional/ShutdownIT.java > 5b83944 > > test/src/test/java/org/apache/accumulo/test/functional/SimpleBalancerFairnessIT.java > 13fea04 > > test/src/test/java/org/apache/accumulo/test/functional/SparseColumnFamilyIT.java > a0d62b0 > test/src/test/java/org/apache/accumulo/test/functional/SplitIT.java 9601de2 > test/src/test/java/org/apache/accumulo/test/functional/SplitRecoveryIT.java > d9de5d1 > test/src/test/java/org/apache/accumulo/test/functional/StartIT.java dff47c4 > test/src/test/java/org/apache/accumulo/test/functional/TableIT.java 26021aa > test/src/test/java/org/apache/accumulo/test/functional/TabletIT.java > 9f7333d > test/src/test/java/org/apache/accumulo/test/functional/TimeoutIT.java > d949a5d > test/src/test/java/org/apache/accumulo/test/functional/VisibilityIT.java > 044a29d > test/src/test/java/org/apache/accumulo/test/functional/WriteAheadLogIT.java > 1c31c2f > test/src/test/java/org/apache/accumulo/test/functional/WriteLotsIT.java > ec481dd > test/src/test/java/org/apache/accumulo/test/functional/ZooCacheIT.java > 9b089a1 > > test/src/test/java/org/apache/accumulo/test/functional/ZookeeperRestartIT.java > fe6c949 > > Diff: https://reviews.apache.org/r/20331/diff/ > > > Testing > ------- > > unit tests pass. flexed scale by tuning down ReadWriteIT and running it w/ > and without scale factor to compensate. > > > Thanks, > > Sean Busbey > >
