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

Store HTML content in file during the XML parsing #16

Conversation

antoine-aumjaud
Copy link
Contributor

The HTML contents was saved in memory and then stored in files once the whole result XML file has been parsed.
To avoid OOM, I write the files during the XML parsing.

I think we could close JENKINS-15863 & JENKINS-13936.

Note : during my tests, I've detected that the "Detail" link is not working in case of test which is not a suite (I think taht it is linked to pull request 10).

@cloudbees-pull-request-builder

plugins » fitnesse-plugin #27 SUCCESS
This pull request looks good

@antoine-aumjaud antoine-aumjaud changed the title Store html content in file when the xml is parsed Store html content in file when the xml has been parsed Aug 28, 2014
@antoine-aumjaud antoine-aumjaud changed the title Store html content in file when the xml has been parsed Store HTML content in file during the XML parsing Aug 28, 2014
@jenkinsadmin
Copy link
Member

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

@radkin
Copy link

radkin commented Aug 28, 2014

I ran the new plugin snapshot on a live Jenkins instance. It created a build.xml artifact, but there was no link for results.

@antoine-aumjaud
Copy link
Contributor Author

Hi,
Thanks for your quick tests and reply.
What do you mean by "no link"?
I have not changed the GUI so:

  • if you have not the link "Lastest Fitnesse result" => I don't know why,
  • if you have not the link to the HTML content (the detail of a test), I
    don't understand, because the code always put a link.
    BUT since last version (pull request changes to overcome performance issues with huge fitnesse result files #10) there is no more link on the test
    name but a new link in a new column nammed Detail.
    Maybe have you not seen this column?
    Regards

@radkin
Copy link

radkin commented Aug 29, 2014

Sorry I wasn't very specific. Here are some details.
The job doesn't have a visible fitnesse link in the far upper left where they used to live with version 1.9. Per your comment, I looked for a details section and didn't see it.
There is still a link for the whole job (before you drill down to a specific job) that goes here http:///jenkins/job//lastCompletedBuild/fitnesseReport/
When I click on the link above it takes me to the latest completed build page http:///jenkins/job//lastCompletedBuild
It is still possible that I simply haven't found the "details" column. Also, this could be an environment (not code) issue and I am looking into that possibility.
Thanks

@radkin
Copy link

radkin commented Aug 29, 2014

As you might expect, when I try to go directly to the fitnesseReport for the last job it shows up in our apache tomcat log as a 404, aka "content not found". The directory for that job on disk doesn't have the files found on a normal fitnesse run.

@radkin
Copy link

radkin commented Aug 29, 2014

I confirmed it was an environment issue. Rolled back the config for our test job and it ran with results no problem. Testing again and will provide more results Tuesday 9/2

@radkin
Copy link

radkin commented Sep 4, 2014

We ran the new plugin artifact for several days and with those fitnesse results we were able to see that Eden space heap increased when viewing results as expected. The heap space did not accumulate over time to result in an OOM. As far as we are concerned your changes look great. Thanks Antoine

@radkin
Copy link

radkin commented Sep 9, 2014

👍
ready for merge

antoine-aumjaud added a commit that referenced this pull request Oct 8, 2014
…StoreHtmlContentInFileWhenXMLIsParsed

Store HTML content in file during the XML parsing to avoid OOM
@antoine-aumjaud antoine-aumjaud merged commit 541f4bb into jenkinsci:master Oct 8, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants