[
https://issues.apache.org/jira/browse/DERBY-3248?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12548410
]
Rick Hillegas commented on DERBY-3248:
--------------------------------------
A call to Process.waitFor() sounds like a good idea.
I think it's also a good idea to add a comment to getProcessOutput() noting
that the producer and consumer of the String are both using the platform
default encoding.
A fail() assertion in runServerCommand() sounds good too. Should something
similar be added to NetworkServerTestStartup()?
I think that this test could use NetworkServerTestSetup where it is currently
calling runServerCommand(). NetworkServerTestSetup can pass back the input
stream needed to figure out if the server command ran correctly and that, I
think, is the chief job of runServerCommand().
> SecureServerTest needs cleanup runServerCommand, possibly causing test
> failures when tests are run through ant.
> ---------------------------------------------------------------------------------------------------------------
>
> Key: DERBY-3248
> URL: https://issues.apache.org/jira/browse/DERBY-3248
> Project: Derby
> Issue Type: Bug
> Components: Test
> Affects Versions: 10.4.0.0
> Reporter: Daniel John Debrunner
>
> SecureServerTest.runServerCommand runs a command and then immediately creates
> a String based upon the output stream of the command. This string is then
> checked to see if it contains exception strings. However, I think that at the
> point the string is created there is no guarantee that the command has
> finished (or even started) execution. Thus there is a significant chance that
> the string will be empty (or at least missing exceptions) and thus always
> pass the test. Should the test be waiting for the command to complete using
> Process.waitFor().
> In getProcessOutput the bytes are converted to a String using the default
> encoding, this tends to cause problems for platforms with different encodings
> in test runs. It may be ok here since the output is from the system, if so it
> would be useful to have a comment indicating that the default encoding is ok
> here. (to avoid other tests blindly copying potentially error prone (due to
> encoding issues)code).
> In runServerCommand() if the exec fails the exception is just printed to
> System.err. SHould this instead be a fail() assertion. The test will fail if
> this occurs due to a NPE on the next access of serverProcess, but I think an
> explicit fail() would be more readable.
> I'm seeing 9 test failures when running derbynet._Suite through ant, on
> windows which I don't see if I run it directly using the swing test runner.
> This is one of the two tests that potentially cause issues (ie. removing it
> from the suite makes the failures disappear).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
