FLUME-627: Make all temp files created by File.createTempFile or FileUtil.mktmpdir live in one dir.

Review Request #1736 - Created May 3, 2011 and submitted

Jonathan Hsieh
old-flume
flume-627
flume
esammer, phunt
Recent build failures have been attributed to permissions errors due to poor hygiene with temp files in tests.  This puts most temp files in the testsuite into one directory per class, and allows for an environment var so that forked test runs can all write to the same tmp dir.

- Replace instances of File.createTempFile with FileUtil.createTempFile.

There are two interesting files (TestFileUtil, FileUtil) and the rest are essentially trivial changes.

Remaining temp file locations are from other subsystems that choose their own temp file locations (zk), or hard coded tmp file locations, and are in separate patches.
Tests pass (except for a flaky, and the broken jetty tests)

Also, using FLUME_TMP_DIR environment like puts most tmp files into specified dir.

$ FLUME_TMP_DIR=foobar mvn package

Puts almost all temp files into /tmp/foobar/*

  1. Up to you if you want to address the static stuff. It would make me happy. ;)
  2. flume-core/src/main/java/com/cloudera/util/FileUtil.java (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
    I'm (almost) always against these types of things in static blocks. They're untestable as a contained unit. Whether they really need to be tested as an atom is another story (they probably don't).
    
    The problem with this is that when makeDirs() runs depends on when FileUtils is class loaded which is really sketchy.
    1. I think I'll meet you halfway.  I'll pull out the code in the static block into a static method and make a test for that.  There will still be a static block, but it will only have the call to the static method.
      
    2. ok, building a test for it is more effort than I would like to spend, will do minor refactor and push.
  3. 
      
Loading...