+1 for using @Rule for any file creation...

-Anil.


On Mon, Jan 8, 2018 at 11:26 AM, Patrick Rhomberg <prhomb...@pivotal.io>
wrote:

> The ClusterStartupRule, LocatorStarterRule, and ServerStarterRule rules all
> have withTempWorkingDir methods that should take care of this, as well.
> These temporary directories should be removed as part of the rules @After
>  invocation.
>
> On Mon, Jan 8, 2018 at 10:23 AM, Jinmei Liao <jil...@pivotal.io> wrote:
>
> > +1 for always using absolute path in our product code.
> >
> > Also the server/locator launchers should be able to take a working-dir as
> > parameter to store all the stat/logs/config files.
> >
> > On Fri, Jan 5, 2018 at 3:49 PM, Kirk Lund <kl...@apache.org> wrote:
> >
> > > The from should be:
> > >
> > >     this.viewFile = new File("locator" + server.getPort() +
> "view.dat");
> > >
> > > On Fri, Jan 5, 2018 at 3:48 PM, Kirk Lund <kl...@apache.org> wrote:
> > >
> > > > Just to help facilitate the discussion, here's a pull request that
> > > changes
> > > > GMSLocator from:
> > > >
> > > >     this.viewFile = new File(System.getProperty("user.dir"),
> > "locator" +
> > > > server.getPort() + "view.dat");
> > > >
> > > > ...to:
> > > >
> > > >     this.viewFile = new File(System.getProperty("user.dir"),
> > "locator" +
> > > > server.getPort() + "view.dat");
> > > >
> > > > To allow the new test LocatorViewFileTemporaryFolderDUnitTest to
> > > redirect
> > > > the locator view dat file to a JUnit TemporaryFolder.
> > > >
> > > > The only other way I can think of to this is to introduce a new Geode
> > > > property for "current-directory" which a test could specify. That
> > > property
> > > > value would have to be pushed down to every class that creates a new
> > > File.
> > > >
> > > > Pull request: https://github.com/apache/geode/pull/1243
> > > >
> > > > On Fri, Jan 5, 2018 at 3:32 PM, Kirk Lund <kl...@apache.org> wrote:
> > > >
> > > >> Any calls such as:
> > > >>
> > > >>     File file = new File("myfile");
> > > >>
> > > >> ...results in creating a file in the current working directory of
> > > >> IntelliJ or Gradle when executing the test.
> > > >>
> > > >> I previously made a change to Gfsh so that tests can pass in a
> parent
> > > >> directory which will be used instead. This allowed me to change all
> of
> > > the
> > > >> Gfsh tests to write to a JUnit TemporaryFolder.
> > > >>
> > > >> This allows us to clean up ALL file artifacts produced from a test
> and
> > > >> also allows us to avoid file-based pollution from one test to
> another.
> > > >>
> > > >> I'd like to propose that we either always pass a parent directory
> > into a
> > > >> component that produces file artifacts or change all of our code
> from
> > > using
> > > >> the constructor File(String pathname) to using the constructor
> > > File(String
> > > >> parent, String child).
> > > >>
> > > >> That 2nd approach would involve changing lines like this:
> > > >>
> > > >>     File file = new File("myfile");
> > > >>
> > > >> ...to:
> > > >>
> > > >>     File file = new File(System.getProperty("user.dir"), "myfile");
> > > >>
> > > >> Then if you add this line to a test:
> > > >>
> > > >> System.setProperty("user.dir", temporaryFolder.getRoot().getA
> > > >> bsolutePath());
> > > >>
> > > >> ...you're able to redirect all file artifacts to the JUnit
> > > >> TemporaryFolder.
> > > >>
> > > >> If we don't make this change to product code, then I really don't
> > think
> > > >> we should be manipulating "user.dir" in ANY of our tests because the
> > > >> results are rather broken.
> > > >>
> > > >> If we don't like using "user.dir" then we could devise our own Geode
> > > >> system property for "the current working directory." Honestly, I
> don't
> > > care
> > > >> what system property we use or don't use, but I really want to have
> > ALL
> > > >> file artifacts properly cleaned and deleted after every test. And I
> > > really
> > > >> want to prevent file-based test pollution.
> > > >>
> > > >
> > > >
> > >
> >
> >
> >
> > --
> > Cheers
> >
> > Jinmei
> >
>

Reply via email to