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.

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

We need to flush our FileWriter's created in cucumber-core #173

Closed
wants to merge 1 commit into from
Closed

We need to flush our FileWriter's created in cucumber-core #173

wants to merge 1 commit into from

Conversation

dkowis
Copy link
Member

@dkowis dkowis commented Jan 25, 2012

System.exit(int) doesn't appear to ensure that our FileWriters are flushed, and so any output configured to go to files, doesn't.

I found this bug (#172) when running against our project, barfing out JSON to a file to see what I could do with that in terms of reporting, and or giving it to someone else for ingestion.

Then, noticing that it didn't output all the data, I tried adding it to the jruby project in cucumber-jvm, just to have a smallish starting point. Turns out nothing was making it into the output file, but if I barfed to the console, everything showed up! Our file writers weren't being flushed.

This adds a shutdown hook only for the filewriters we create, and calls flush on them. I suppose it could call close, but flushing them seems to get the job done.

Opinions? Should we be doing this entirely differently?

@aslakhellesoy
Copy link
Contributor

The bug was in Gherkin's own formatters. At the end of the run, Formatter.done() is called, it just wasn't doing the appropriate flush()/close().

@aslakhellesoy
Copy link
Contributor

I spoke too soon. I first did cucumber/gherkin@ac0cc51 and then reverted it with cucumber/gherkin@f432f28

We definitely need to close the streams in cucumber-jvm. I'm not happy with the shutdown hook approach though - shutdown hooks are not guaranteed to run.

@aslakhellesoy
Copy link
Contributor

@dkowis Take a look at how I did it. I think that's a cleaner solution.

@dkowis
Copy link
Member Author

dkowis commented Jan 25, 2012

@aslakhellesoy Totally. That's a much cleaner solution. I wasn't sure that making changes to gherkin was a good idea, but I like this a lot. Works for me.

@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants