Hi David,
Just to clarify one point, most of the tests that use OutputAnalyzer do
not display process output unless there is an error. So part of the
decision here with LingeredApp is when to display the output. Currently
the stdout is captured, but not displayed, unless the tests does the
work to display it, which none do. Currently stderr goes to the console.
Note that some negative tests actually cause some expected stderr
output, although the tests don't check for it.
One thought I just had is to create an async option for OutputAnalyzer
so it doesn't block until the process exits. Basically that means
splitting ProcessTools.getOutput() so it doesn't block. What I currently
have is essentially doing that. It copies ProcessTools.getOutput(),
splitting it into two parts. But all this logic is in LingeredApp, and
of course doesn't have any of the output error checking support that
OutputAnalyzer, which might be useful for LingeredApp. For example, the
negative tests only test that launching the app failed. They could be
improved by checking for specific error output.
Chris
On 3/17/18 12:11 AM, David Holmes wrote:
I'm afraid I'm losing track of this change.
The key thing is that we should not have a test that launches any
other process for which we can not see the output of that process.
David
On 17/03/2018 7:48 AM, Chris Plummer wrote:
On 3/16/18 1:25 PM, serguei.spit...@oracle.com wrote:
Hi Chris,
Thank you for taking care about this issue!
On 3/16/18 11:20, Chris Plummer wrote:
Hi,
I've resolved the issues I had before with not seeing all the
stderr output when I tried to capture it. What I'd like to do now
is have us decide how the output should be handled from the
perspective a LingeredApp user (driver app). Currently all
LingeredApp stdout is captured and gets be returned the the driver
app by calling app.getAppOutput(). It does not appear in the .jtr
file, but the test would have the option of dumping it there it it
cared to. Only one test uses app.getAppOutput(). Currently all the
LingeredApp stderr is redirected to the console, so it does not
appear in the .jtr file.
Just a general comment to make sure I understand it and ensure we
are in sync.
It seems much more safe to always have both stdout and stderr
outputs present in the .jtr automatically file independently of of
what the test does.
So how do we want this changed? Some possibilities are:
(1) capture stderr just like stdout currently is, and leave is up
the the driver app to decide if it wants to display it (after the
app terminates).
It does not look good to me (see above) but maybe I'm missing
something important here.
(2) capture stderr just like stdout currently is, but have
LingeredApp automatically send captured output to driver app's
stdout and stderr (after the app terminates).
The stdout and std err will be separated in this case, right?
Do you have a webrev for this?
I currently have it working like this, although I need to fix
LingeredApp.getAppOutput(). I had to make it return a single String
instead of a List of Strings, so this breaks the one test that uses
this API. It's easily fixed. Just haven't gotten around to it yet.
(3) send the LingeredApp's stdout and stderr to the driver app's
stdout as it is being captured (this was the original fix Igor
suggested and the webrev supported). A minor alternative to this is
to keep the two streams separated instead of sending both to stdout.
Let me know what you think. I'm inclined to go with 2, especially
since normally there is little to no output from the LingeredApp.
The choice (2) looks good enough.
Not sure it is that important to have output from stdout and stderr
sync'ed
but is is important to have the stderr present in the .jtr
automatically.
The choice (3) looks even better if it is going to work well.
This is basically what the original webrev did. It sent LingeredApp's
stderr and stdout to the the driver apps stdout. It's a 1 word change
to make it send stderr to stderr. I think it has a bug though that
did not manifest itself. It seems the new copy() code that is
capturing stdout would be contending with the existing InputGlobbler
code that is doing the same. I would need to fix this to make sure
LingeredApp.getAppOutput() still returns all the apps stdout output.
Chris
Not sure, it is really necessary.
Thanks,
Serguei
BTW, here's the CR and original webrev for reference:
https://bugs.openjdk.java.net/browse/JDK-8198655
http://cr.openjdk.java.net/~cjplummer/8198655/webrev.00/webrev/
thanks,
Chris