Skip to content

Commit

Permalink
[Core, TestNg] Clean up stream closing
Browse files Browse the repository at this point in the history
  Refactored the code around streams so it follows these rules:
   - Streams should be closed by their owners
   - A stream is owned by its creator or when provided as a constructor argument
   - Stream ownership should be the smallest possible unit. The method, class, process, system.
   - System.out and System.error may never be closed

 Fixes #1108
  • Loading branch information
mpkorstanje committed Jul 9, 2017
1 parent 51f0c9b commit 33e4dde
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public NiceAppendable println(CharSequence csq) {
public void close() {
try {
tryFlush();
if (out instanceof Closeable && out != System.out && out != System.err) {
if (out instanceof Closeable) {
((Closeable) out).close();
}
} catch (IOException e) {
Expand Down
50 changes: 36 additions & 14 deletions core/src/main/java/cucumber/runtime/formatter/HTMLFormatter.java
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,12 @@ public void receive(TestRunFinished event) {
};

public HTMLFormatter(URL htmlReportDir) {
this.htmlReportDir = htmlReportDir;
this(htmlReportDir, null);
}

public HTMLFormatter(URL htmlReportDir, NiceAppendable jsOut) {
this(htmlReportDir);
this.jsOut = jsOut;
HTMLFormatter(URL htmlReportDir, NiceAppendable optionalJsOut) {
this.htmlReportDir = htmlReportDir;
this.jsOut = optionalJsOut; // For testing purposes
}

@Override
Expand Down Expand Up @@ -188,7 +188,7 @@ private void handleEmbed(EmbedEvent event) {
String extension = MIME_TYPES_EXTENSIONS.get(mimeType);
if (extension != null) {
StringBuilder fileName = new StringBuilder("embedded").append(embeddedIndex++).append(".").append(extension);
writeBytesAndClose(event.data, reportFileOutputStream(fileName.toString()));
writeBytesToURL(event.data, toUrl(fileName.toString()));
jsFunctionCall("embedding", mimeType, fileName);
}
}
Expand Down Expand Up @@ -463,50 +463,72 @@ private void copyReportFiles() {
if (textAssetStream == null) {
throw new CucumberException("Couldn't find " + textAsset + ". Is cucumber-html on your classpath? Make sure you have the right version.");
}
String baseName = new File(textAsset).getName();
writeStreamAndClose(textAssetStream, reportFileOutputStream(baseName));
String fileName = new File(textAsset).getName();
writeStreamToURL(textAssetStream, toUrl(fileName));
}
}

private URL toUrl(String fileName) {
try {
return new URL(htmlReportDir, fileName);
} catch (IOException e) {
throw new CucumberException(e);
}
}

private void writeStreamAndClose(InputStream in, OutputStream out) {
private static void writeStreamToURL(InputStream in, URL url) {
OutputStream out = createReportFileOutputStream(url);

byte[] buffer = new byte[16 * 1024];
try {
int len = in.read(buffer);
while (len != -1) {
out.write(buffer, 0, len);
len = in.read(buffer);
}
out.close();
} catch (IOException e) {
throw new CucumberException("Unable to write to report file item: ", e);
} finally {
closeQuietly(out);
}
}

private void writeBytesAndClose(byte[] buf, OutputStream out) {
private static void writeBytesToURL(byte[] buf, URL url) throws CucumberException {
OutputStream reportFileOutputStream = createReportFileOutputStream(url);
try {
out.write(buf);
reportFileOutputStream.write(buf);
} catch (IOException e) {
throw new CucumberException("Unable to write to report file item: ", e);
} finally {
closeQuietly(reportFileOutputStream);
}
}

private NiceAppendable jsOut() {
if (jsOut == null) {
try {
jsOut = new NiceAppendable(new OutputStreamWriter(reportFileOutputStream(JS_REPORT_FILENAME), "UTF-8"));
jsOut = new NiceAppendable(new OutputStreamWriter(createReportFileOutputStream(toUrl(JS_REPORT_FILENAME)), "UTF-8"));
} catch (IOException e) {
throw new CucumberException(e);
}
}
return jsOut;
}

private OutputStream reportFileOutputStream(String fileName) {
private static OutputStream createReportFileOutputStream(URL url) {
try {
return new URLOutputStream(new URL(htmlReportDir, fileName));
return new URLOutputStream(url);
} catch (IOException e) {
throw new CucumberException(e);
}
}

private static void closeQuietly(OutputStream out) {
try {
out.close();
} catch (IOException ignored) {
// go gentle into that good night
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import cucumber.runtime.io.ResourceLoaderClassFinder;
import cucumber.runtime.model.CucumberFeature;

import java.io.PrintStream;
import java.util.ArrayList;
import java.util.List;

Expand All @@ -36,7 +37,12 @@ public TestNGCucumberRunner(Class clazz) {
RuntimeOptionsFactory runtimeOptionsFactory = new RuntimeOptionsFactory(clazz);
runtimeOptions = runtimeOptionsFactory.create();

reporter = new TestNgReporter(System.out);
reporter = new TestNgReporter(new PrintStream(System.out) {
@Override
public void close() {
// We have no intention to close System.out
}
});
ClassFinder classFinder = new ResourceLoaderClassFinder(resourceLoader, classLoader);
resultListener = new FeatureResultListener(runtimeOptions.isStrict());
runtime = new Runtime(resourceLoader, classFinder, classLoader, runtimeOptions);
Expand Down

0 comments on commit 33e4dde

Please sign in to comment.