Skip to content

Commit

Permalink
Issue #5684 - Window's test overhaul
Browse files Browse the repository at this point in the history 
+ Cleanup FileBufferedResponseHandlerTest expectations on Windows.

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
  • Loading branch information
@joakime
joakime committed Jul 30, 2021
1 parent 7741ecc commit 87912d8
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 25 deletions.
28 changes: 22 additions & 6 deletions jetty-server/src/main/java/org/eclipse/jetty/server/handler/FileBufferedResponseHandler.java
View file
Expand Up @@ -111,21 +111,38 @@ public void resetBuffer()
BufferedInterceptor.super.resetBuffer();
}

private void closeFileOutput()
{
if (_fileOutputStream != null)
{
try
{
_fileOutputStream.flush();
}
catch (IOException e)
{
LOG.debug("flush failure", e);
}
IO.close(_fileOutputStream);
_fileOutputStream = null;
}
}

protected void dispose()
{
IO.close(_fileOutputStream);
_fileOutputStream = null;
closeFileOutput();
_aggregating = null;

if (_filePath != null)
{
try
{
Files.delete(_filePath);
Files.deleteIfExists(_filePath);
}
catch (Throwable t)
{
LOG.warn("Could not delete file {}", _filePath, t);
LOG.debug("Could not immediately delete file (delaying to jvm exit) {}", _filePath, t);
_filePath.toFile().deleteOnExit();
}
_filePath = null;
}
Expand Down Expand Up @@ -192,8 +209,7 @@ private void commit(Callback callback)

try
{
_fileOutputStream.close();
_fileOutputStream = null;
closeFileOutput();
}
catch (Throwable t)
{
Expand Down
78 changes: 59 additions & 19 deletions ...erver/src/test/java/org/eclipse/jetty/server/handler/FileBufferedResponseHandlerTest.java
View file
Expand Up @@ -50,12 +50,16 @@
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.toolchain.test.FS;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDir;
import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension;
import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.OS;
import org.junit.jupiter.api.extension.ExtendWith;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
Expand All @@ -65,10 +69,13 @@
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

@ExtendWith(WorkDirExtension.class)
public class FileBufferedResponseHandlerTest
{
private static final Logger LOG = Log.getLogger(FileBufferedResponseHandlerTest.class);

public WorkDir _workDir;

private final CountDownLatch _disposeLatch = new CountDownLatch(1);
private Server _server;
private LocalConnector _localConnector;
Expand All @@ -79,8 +86,7 @@ public class FileBufferedResponseHandlerTest
@BeforeEach
public void before() throws Exception
{
_testDir = MavenTestingUtils.getTargetTestingPath(FileBufferedResponseHandlerTest.class.getName());
FS.ensureDirExists(_testDir);
_testDir = _workDir.getEmptyPathDir();

_server = new Server();
HttpConfiguration config = new HttpConfiguration();
Expand Down Expand Up @@ -114,8 +120,6 @@ protected void dispose()
_bufferedHandler.getPathIncludeExclude().exclude("*.exclude");
_bufferedHandler.getMimeIncludeExclude().exclude("text/excluded");
_server.setHandler(_bufferedHandler);

FS.ensureEmpty(_testDir);
}

@AfterEach
Expand Down Expand Up @@ -180,8 +184,13 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
assertThat(responseContent, containsString("Committed: false"));
assertThat(responseContent, containsString("NumFiles: 1"));

assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
assertThat(getNumFiles(), is(0));
// Unable to verify file deletion on windows, as immediate delete not possible.
// only after a GC has occurred.
if (!OS.WINDOWS.isCurrentOs())
{
assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
assertThat(getNumFiles(), is(0));
}
}

@Test
Expand Down Expand Up @@ -274,8 +283,13 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
assertThat(responseContent, containsString("Committed: false"));
assertThat(responseContent, containsString("NumFiles: 1"));

assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
assertThat(getNumFiles(), is(0));
// Unable to verify file deletion on windows, as immediate delete not possible.
// only after a GC has occurred.
if (!OS.WINDOWS.isCurrentOs())
{
assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
assertThat(getNumFiles(), is(0));
}
}

@Test
Expand Down Expand Up @@ -306,8 +320,13 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
assertThat(responseContent, not(containsString("writtenAfterClose")));
assertThat(responseContent, containsString("NumFiles: 1"));

assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
assertThat(getNumFiles(), is(0));
// Unable to verify file deletion on windows, as immediate delete not possible.
// only after a GC has occurred.
if (!OS.WINDOWS.isCurrentOs())
{
assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
assertThat(getNumFiles(), is(0));
}
}

@Test
Expand Down Expand Up @@ -368,8 +387,13 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
assertThat(response.getStatus(), is(HttpStatus.OK_200));
assertThat(responseContent, containsString("NumFiles: 0"));

assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
assertThat(getNumFiles(), is(0));
// Unable to verify file deletion on windows, as immediate delete not possible.
// only after a GC has occurred.
if (!OS.WINDOWS.isCurrentOs())
{
assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
assertThat(getNumFiles(), is(0));
}
}

@Test
Expand Down Expand Up @@ -405,12 +429,18 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
// Resetting the response buffer will delete the file.
assertThat(response.getStatus(), is(HttpStatus.OK_200));
assertThat(responseContent, not(containsString("THIS WILL BE RESET")));

assertThat(responseContent, containsString("NumFilesBeforeReset: 1"));
assertThat(responseContent, containsString("NumFilesAfterReset: 0"));
assertThat(responseContent, containsString("NumFilesAfterWrite: 1"));

assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
assertThat(getNumFiles(), is(0));
// Unable to verify file deletion on windows, as immediate delete not possible.
// only after a GC has occurred.
if (!OS.WINDOWS.isCurrentOs())
{
assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
assertThat(getNumFiles(), is(0));
}
}

@Test
Expand Down Expand Up @@ -484,8 +514,13 @@ public boolean content(ByteBuffer ref)
assertThat(response.get("FileSize"), is(Long.toString(fileSize)));
assertThat(received.get(), is(fileSize));

assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
assertThat(getNumFiles(), is(0));
// Unable to verify file deletion on windows, as immediate delete not possible.
// only after a GC has occurred.
if (!OS.WINDOWS.isCurrentOs())
{
assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
assertThat(getNumFiles(), is(0));
}
}

@Test
Expand Down Expand Up @@ -564,9 +599,14 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
Throwable error = errorFuture.get(5, TimeUnit.SECONDS);
assertThat(error.getMessage(), containsString("intentionally throwing from interceptor"));

// All files were deleted.
assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
assertThat(getNumFiles(), is(0));
// Unable to verify file deletion on windows, as immediate delete not possible.
// only after a GC has occurred.
if (!OS.WINDOWS.isCurrentOs())
{
// All files were deleted.
assertTrue(_disposeLatch.await(5, TimeUnit.SECONDS));
assertThat(getNumFiles(), is(0));
}
}

@Test
Expand Down

0 comments on commit 87912d8

Please sign in to comment.