-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52510/#review153104
-----------------------------------------------------------



Ahh now I see what is going on. BucketWriter instantiate a Clock class which is 
in turn used in the file name counter. The test first instantiates the 
BucketWriter - which initialize the filename counter -then overrides the Clock 
but since Clock is not used in BucketWriter directly - only via the file name 
counter at instantiation time so it won't be updated with the override - the 
test failes as it checks the file name.
So actually a single extra line into the `BucketWriter.setClock(Clock clock)` 
method would solve the issue:
```
  void setClock(Clock clock) {
    this.clock = clock;
    this.fileExtensionCounter.set(clock.currentTimeMillis());
  }
```
This way the test would update the relevant information - file name counter - 
which then in turn can be checked.

Could you please consider simplifying your change?

- Attila Simon


On Oct. 4, 2016, 11:11 a.m., Balázs Donát Bessenyei wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52510/
> -----------------------------------------------------------
> 
> (Updated Oct. 4, 2016, 11:11 a.m.)
> 
> 
> Review request for Flume.
> 
> 
> Bugs: FLUME-3002
>     https://issues.apache.org/jira/browse/FLUME-3002
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> testFileSuffixNotGiven (and probably a few other tests) are flaky
> 
> 
> Diffs
> -----
> 
>   
> c/flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/BucketWriter.java
>  b096410 
>   
> c/flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/TestBucketWriter.java
>  742deb0 
> 
> Diff: https://reviews.apache.org/r/52510/diff/
> 
> 
> Testing
> -------
> 
> mvn clean install runs successfully in flume-ng-sinks
> 
> 
> Thanks,
> 
> Balázs Donát Bessenyei
> 
>

Reply via email to