RFR(xs): 8080925: Make error log write timeout parameter configurable

[Coleen Phillimore]

On 6/25/15 2:31 AM, Thomas Stüfe wrote:
> Hi Coleen,
>
> thank you for reviewing! I agree that we have too many command line 
> switches and I am guilty because I introduced a number of switches 
> which test Error handling: TestCrashInErrorHandler, 
> TestSafeFetchInErrorHandler, now TestTimeoutInErrorHandler.
>
> We could mitigate this by combining them in one switch. We already do 
> this for the "ErrorHandlerTest" switch which takes a number 1-15 to 
> control the way we die. Only problem is the naming: 
> "TestInErrorHandler" or similar would fit, but we already have 
> "ErrorHandlerTest" (which maybe could be renamed to "DieWith" or 
> "CrashWith").
>

So I'm confused why you added TestSafeFetchInErrorHandler instead of 
adding another numeric value to TestCrashInErrorHandler for testing 
SafeFetch.  The TestCrashInErrorHandler seems like a decent name and 
useful additional flag.  I had to do some reading to figure out the 
difference between ErrorHandlerTest and TestCrashInErrorHandler but 
after that, it seems to make sense, although it looks like you only give 
0 or 1 for it, why not have SafeFetch be 2?

Thanks,
Coleen

> Thomas
>
>
>
> On Wed, Jun 24, 2015 at 5:44 PM, Coleen Phillimore 
> <coleen.phillimore at oracle.com <mailto:coleen.phillimore at oracle.com>> 
> wrote:
>
>
>     Hi Thomas,
>
>     This change looks okay, I guess.  I've been avoiding tests that
>     crash the JVM because I don't know if this will mess up test
>     execution in any way.   Also I guess I don't see any other way to
>     add the test without a command line parameter, like
>     TestSafeFetchInErrorHandler , but I feel like we're swamped in
>     command line parameters!   But you just got rid of one that was
>     broken, so I guess this is fine.
>
>     I can sponsor your change if someone from our SQE group okays the
>     test.
>
>     Thanks,
>     Coleen
>
>
>     On 6/24/15 11:30 AM, Thomas Stüfe wrote:
>
>         Hi,
>
>         This change is in review limbo since a month...
>
>         Could I have a second reviewer, please, and a sponsor?
>
>         Or, if the change is not wanted, a reason why?
>
>         Thanks a lot,
>
>         Thomas
>
>
>         On Mon, Jun 15, 2015 at 2:58 PM, Thomas Stüfe
>         <thomas.stuefe at gmail.com <mailto:thomas.stuefe at gmail.com>>
>         wrote:
>
>             Hi all,
>
>             may I have a second review for this tiny change? I also
>             need a sponsor.
>
>             Thanks!
>
>             On Fri, May 22, 2015 at 4:21 PM, Thomas Stüfe
>             <thomas.stuefe at gmail.com <mailto:thomas.stuefe at gmail.com>>
>             wrote:
>
>                 Hi Staffan,
>
>                 thanks!
>
>                 ...Thomas
>
>                 On Fri, May 22, 2015 at 1:55 PM, Staffan Larsen <
>                 staffan.larsen at oracle.com
>                 <mailto:staffan.larsen at oracle.com>> wrote:
>
>                     Looks good to me.
>
>                     A small nit in globals.hpp:925: "an timeout” -> “a
>                     timeout”. No need for
>                     a new webrev if you fix this.
>
>                     Thanks,
>                     /Staffan
>
>                         On 22 maj 2015, at 13:33, Thomas Stüfe
>                         <thomas.stuefe at gmail.com
>                         <mailto:thomas.stuefe at gmail.com>>
>
>                     wrote:
>
>                         Hi,
>
>                         please review this small change:
>
>                         http://cr.openjdk.java.net/~stuefe/webrevs/8080925/webrev.00/webrev/
>                         <http://cr.openjdk.java.net/%7Estuefe/webrevs/8080925/webrev.00/webrev/>
>
>                         https://bugs.openjdk.java.net/browse/JDK-8080925
>
>                         This is a small addition which makes the
>                         timeout to write error log
>
>                     files
>
>                         configurable.
>
>                         Thanks & Kind Regards,
>
>                         Thomas
>
>
>
>