RFR: 8285878: [TestBug] LocalStorageTest and UserDataDirectoryTest don't always cleanup data dirs

[Kevin Rushforth]

On Wed, 19 Apr 2023 13:25:07 GMT, Jay Bhaskar <jbhaskar at openjdk.org> wrote:

> Issue: "LocalStorageTest and UserDataDirectoryTest don't always cleanup data dirs"
> Solution:
> 1. All data directories must only be created under the "build" dir. In addition to being a best practice, at least they will be cleaned up by "gradle clean"
> 2. There should be an @AfterClass method that deletes the data dirs
> 3. There should be an @BeforeClass method that also deletes the data dirs prior to running the tests in case it wasn't cleaned up previously

I haven't tested it yet, but I did leave a couple comments.

modules/javafx.web/src/test/java/test/javafx/scene/web/LocalStorageTest.java line 45:

> 43: 
> 44:     private static final File LOCAL_STORAGE_DIR_PATH = new File("build/");
> 45:     private static final File LOCAL_STORAGE_DIR = new File("build/localstorage");

I don't see the need for two variables here. Why not use the updated value of `LOCAL_STORAGE_DIR` everywhere?

modules/javafx.web/src/test/java/test/javafx/scene/web/LocalStorageTest.java line 51:

> 49:             for (File f : file.listFiles()) {
> 50:                 deleteRecursively(f);
> 51:                 f.delete();

This will miss deleting the original directory passed into this method. What was wrong with the old way of doing it?

modules/javafx.web/src/test/java/test/javafx/scene/web/LocalStorageTest.java line 84:

> 82:         final WebEngine webEngine = getEngine();
> 83:         webEngine.setJavaScriptEnabled(true);
> 84:         webEngine.setUserDataDirectory(LOCAL_STORAGE_DIR_PATH);

I think this change should be reverted (and similarly in other places)

-------------

PR Review: https://git.openjdk.org/jfx/pull/1100#pullrequestreview-1392117603
PR Review Comment: https://git.openjdk.org/jfx/pull/1100#discussion_r1171374713
PR Review Comment: https://git.openjdk.org/jfx/pull/1100#discussion_r1171376803
PR Review Comment: https://git.openjdk.org/jfx/pull/1100#discussion_r1171377563