Re: RFR 8200706: Better cleanup for open/test/jdk/java/lang/ProcessBuilder/DestroyTest.java

[Roger Riggs]

Thanks Lance, Paul,

I will refactor the code, update in place:

http://cr.openjdk.java.net/~rriggs/webrev-clean-destroy-8200706/

And I am retesting.

Thanks, Roger

On 4/6/2018 12:15 PM, Paul Sandoz wrote:



On Apr 6, 2018, at 6:50 AM, Roger Riggs  wrote:

Please review an intermittent test bug/cleanup improvement that places temporary
files in the directory that is auto-cleaned by jtreg.

Webrev:
  http://cr.openjdk.java.net/~rriggs/webrev-clean-destroy-8200706/


Looks ok, while you are there, and up to you, you might consider a little 
refactor of the if/else so there is no repetition of the temp file creation 
code since it’s the same on all but windows e.g:

   if (osName.startsWith("Windows")) {
 return new WindowsTest();
   }

   File userDir = new File(System.getProperty("user.dir", "."));
   File tmpFile = File.createTempFile("ProcessTrap-", ".sh”, userDir));
   if (osName.startsWith("Linux") == true) {
 return return new UnixTest(tempFile);
   } else if …
   ...
   return null;

Paul.


Thanks, Roger

Re: RFR 8200706: Better cleanup for open/test/jdk/java/lang/ProcessBuilder/DestroyTest.java

[Lance Andersen]

Hi Roger,

I think what you have is OK.


Best
Lance


> On Apr 6, 2018, at 9:50 AM, Roger Riggs  wrote:
> 
> Please review an intermittent test bug/cleanup improvement that places 
> temporary
> files in the directory that is auto-cleaned by jtreg.
> 
> Webrev:
>  http://cr.openjdk.java.net/~rriggs/webrev-clean-destroy-8200706/
> 
> Thanks, Roger
> 

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 

Re: RFR 8200706: Better cleanup for open/test/jdk/java/lang/ProcessBuilder/DestroyTest.java

[Paul Sandoz]

> On Apr 6, 2018, at 6:50 AM, Roger Riggs  wrote:
> 
> Please review an intermittent test bug/cleanup improvement that places 
> temporary
> files in the directory that is auto-cleaned by jtreg.
> 
> Webrev:
>  http://cr.openjdk.java.net/~rriggs/webrev-clean-destroy-8200706/
> 

Looks ok, while you are there, and up to you, you might consider a little 
refactor of the if/else so there is no repetition of the temp file creation 
code since it’s the same on all but windows e.g:

  if (osName.startsWith("Windows")) {
return new WindowsTest();
  }

  File userDir = new File(System.getProperty("user.dir", "."));
  File tmpFile = File.createTempFile("ProcessTrap-", ".sh”, userDir));
  if (osName.startsWith("Linux") == true) {
return return new UnixTest(tempFile);
  } else if …
  ...
  return null;

Paul.

> Thanks, Roger
>