Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HtmlFormatter doesn't close Stream for embedded image. #1108

Closed
christopherfrieler opened this issue Mar 17, 2017 · 2 comments · Fixed by #1175
Closed

HtmlFormatter doesn't close Stream for embedded image. #1108

christopherfrieler opened this issue Mar 17, 2017 · 2 comments · Fixed by #1175
Labels
good first issue Good for newcomers

Comments

@christopherfrieler
Copy link

Summary

It looks like the HtmlFormatter forgets to close the FileOutputStream of an image being embedded.

Expected Behavior

The HtmlFormatter should close all streams and there should be no log-error about a stream not being closed.

Current Behavior

Although my tests pass, I get the following exception:

A resource was acquired at attached stack trace but never released. See java.io.Closeable for information on avoiding resource leaks.
                                             java.lang.Throwable: Explicit termination method 'close' not called
                                                 at dalvik.system.CloseGuard.open(CloseGuard.java:180)
                                                 at java.io.FileOutputStream.<init>(FileOutputStream.java:222)
                                                 at java.io.FileOutputStream.<init>(FileOutputStream.java:169)
                                                 at cucumber.runtime.io.URLOutputStream.<init>(URLOutputStream.java:34)
                                                 at cucumber.runtime.io.URLOutputStream.<init>(URLOutputStream.java:24)
                                                 at cucumber.runtime.formatter.HTMLFormatter.reportFileOutputStream(HTMLFormatter.java:229)
                                                 at cucumber.runtime.formatter.HTMLFormatter.embedding(HTMLFormatter.java:157)
                                                 at java.lang.reflect.Method.invoke(Native Method)
                                                 at cucumber.runtime.Utils$1.call(Utils.java:40)
                                                 at cucumber.runtime.Timeout.timeout(Timeout.java:16)
                                                 at cucumber.runtime.Utils.invoke(Utils.java:34)
                                                 at cucumber.runtime.RuntimeOptions$1.invoke(RuntimeOptions.java:294)
                                                 at java.lang.reflect.Proxy.invoke(Proxy.java:813)
                                                 at $Proxy6.embedding(Unknown Source)
                                                 at cucumber.runtime.ScenarioImpl.embed(ScenarioImpl.java:61)
                                                 ...

Possible Solution

In HtmlFormatter there is a private method writeBytesAndClose(byte[], OutputStream), which writes bytes, but does not close the stream as the method-name indicates. Closing the stream could possibly help.

Context & Motivation

If not fixed, I'm scared of a memory-leak, when our test-suite grows.

Your Environment

  • Version used: cucumber-core:1.2.5
  • Operating System and version: Windows 7
  • I'm using cucumber to test an Android-app.
@mpkorstanje
Copy link
Contributor

mpkorstanje commented May 28, 2017 •
edited
Loading

Looks like an easy fix. I think you're best of submitting a PR and possibly writing a test.

Otherwise I would recommend:

Uses the json output and works better overall.

@mpkorstanje mpkorstanje added Bug good first issue Good for newcomers labels May 28, 2017
mpkorstanje added a commit that referenced this issue Jul 9, 2017
@mpkorstanje
[Core, TestNg] Clean up stream closing
33e4dde 
  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
mpkorstanje added a commit that referenced this issue Jul 9, 2017
@mpkorstanje
[Core, TestNg] Clean up stream closing
9edd86b 
  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
@mpkorstanje mpkorstanje mentioned this issue Jul 9, 2017
Merged
6 tasks
mpkorstanje added a commit that referenced this issue Jul 9, 2017
@mpkorstanje
[Core, TestNg] Clean up stream closing
486b82c 
  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
@lock
Copy link

lock bot commented Oct 25, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[Core, TestNg] Clean up stream closing cucumber/cucumber-jvm
2 participants
@mpkorstanje @christopherfrieler