On 19/02/18 1:37 AM, Stefan Bodewig wrote:
On 2018-02-15, Jaikiran Pai wrote:

I'm now done with the initial goals that I had in mind for this
task. I've opened a PR[1] for review. I've included a manual for this
task and it can be currently found here[2]. I would suggest reading
the manual first, before reviewing the PR, since the manual will give
an overall idea of what's being attempted with this task.
Except for the "fork" mode which is available in our existing junit
task and which was was in the TODO list, in my previous mail, the rest
have been implemented in this version. I plan to focus on the "fork"
mode after this task is made available to users and any feedback
received.
I'm through with the manual and can see how the lack of fork is a real
limitation. Setting the classloaders up may be a pain with several
classes of Ant leaking into the test's classpath. IMHO forking new VMs
is the only clean way of running tests. And then there are timeouts or
tests that crash the VM, memory settings and all that.

This is not to say the task shouldn't be merged without the fork
feature, I just expect it to get requested pretty soon :-)
Agreed. I have started the "fork" mode work, but it's probably going to take at least acouple of more weekends to get it usable.


We have 3 pre-defined formatters all of which are capable of receiving
this streamed sysout/syserr data. Each of these do _not_ hold on to
this sysout/syserr data in-memory during the execution and instead
stream it out a temporary file. Once the execution completes and it's
time to present the result, these formatters stream/read back the
content from the temp file and write it out in a formatted manner to
the target report files.
Sounds expensive.
Indeed. In 2 ways actually:

    1. The creation of these files.
    2. The fact that we write it out to these files (although in a separate thread) and then read it while formatting the result, finally.

It's only applicable for result formatters which want to have the sysout and syserr content sent to them, but I expect that most users will want this enabled anyway. So yes, it's expensive.

The alternate approach is perhaps to have this content maintained in-memory till a fixed size limit (relatively large enough but not too much that it will cause OOM) and only create and stream into temporary files if that size limit is exceeded. That way the temp file creation and writing/reading perhaps can be avoided/controlled in most of the cases.

Furthermore, the XML formatter doesn't use DOM and instead is based on
Stax for writing out the report. However, even with all this, there's
one place where I haven't yet been able to avoid reading the whole
generated sysout/syserr data into memory (thus potentially triggering
an OOM) - it's the XMLStreamWriter API which for writing out a CDATA
section (which is what we use for sysout/syserr content) in XML,
expects the entire String object. I'll have to see if there are ways
to avoid it, but I think this still is an improvement since this
loaded content is only held on for a short while in memory, during the
report writing and will be immediately garbage collected once that's
done.
CDATA is really just an implementation detail. The stylesheets should
work with properly escaped nested text as well. So as an alternative you
could escape all special characters with entity references and use
writeCharacters.

Thank you - I hadn't thought of it and my mind was just fixed on the CDATA usage. I have updated that PR to encode the characters and then use writeCharacters to write them out.

Thanks for the review Stefan.

-Jaikiran


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@ant.apache.org
For additional commands, e-mail: dev-h...@ant.apache.org

Reply via email to