> On Aug. 18, 2012, 4:21 a.m., Hari Shreedharan wrote: > > flume-ng-tests/src/test/java/org/apache/flume/test/agent/TestFileChannel.java, > > lines 56-96 > > <https://reviews.apache.org/r/6684/diff/1/?file=142689#file142689line56> > > > > Nit: you could move all this initialization code to a setUp method so > > it can be used for other tests when we add them
Fixed > On Aug. 18, 2012, 4:21 a.m., Hari Shreedharan wrote: > > flume-ng-tests/src/test/java/org/apache/flume/test/agent/TestFileChannel.java, > > line 104 > > <https://reviews.apache.org/r/6684/diff/1/?file=142689#file142689line104> > > > > I'm afraid this will cause the same problem we were facing with the > > integration tests earlier, since it was possible that the test would fail > > because the agent didn't start up in 3 seconds. I don't mind this getting > > committed as is, but please file a followup jira. > > Hari Shreedharan wrote: > I just tried running the other tests(this would not run) on a Macbook > air, and had to put in a much higher timeout than already in the code for the > test to not fail. So 3 seconds seems low to be a general case. > > Hari Shreedharan wrote: > Verified that this test runs, so you can just increase the sleep timeout > to something higher or more deteministic. Increased to 10 sec for now. Will make it more deterministic in a future refactoring. Filed FLUME-1497. Thanks, Hari. > On Aug. 18, 2012, 4:21 a.m., Hari Shreedharan wrote: > > flume-ng-tests/src/test/java/org/apache/flume/test/agent/TestFileChannel.java, > > line 129 > > <https://reviews.apache.org/r/6684/diff/1/?file=142689#file142689line129> > > > > Nit: Why not just read the actual output into memory and then compare > > the two strings. What is done here seems much more expensive - generate > > expected out, write it out, and then read both files back in to compare. > > Fixed - Will ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6684/#review10489 ----------------------------------------------------------- On Aug. 20, 2012, 8:51 p.m., Will McQueen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6684/ > ----------------------------------------------------------- > > (Updated Aug. 20, 2012, 8:51 p.m.) > > > Review request for Flume. > > > Description > ------- > > First integration test for file channel > > > This addresses bug FLUME-1492. > https://issues.apache.org/jira/browse/FLUME-1492 > > > Diffs > ----- > > flume-ng-tests/pom.xml 7d5ebed > > flume-ng-tests/src/test/java/org/apache/flume/test/agent/TestFileChannel.java > e69de29 > flume-ng-tests/src/test/java/org/apache/flume/test/util/StagedInstall.java > 3e7940d > > Diff: https://reviews.apache.org/r/6684/diff/ > > > Testing > ------- > > > Thanks, > > Will McQueen > >
