You can use %n in printf to get a system-specific newline. — Igor
> On Apr 8, 2020, at 8:12 AM, Evgeny Nikitin <evgeny.niki...@oracle.com> wrote: > > Hi David, > > > > You can use printf rather than making a separate call to format. > > Good point, I've missed that method. I'll fix it and ask Igor to change > > the webrev. > > I finally decided to leave it as it is now, since printf doesn't add a > newline and System.lineSeparator() is not shorter than String.format() :). > Please let me know if you disagree. > > Best regards, > Evgeny. > > >> On 2020-04-07 16:09, Evgeny Nikitin wrote: >> Hi David, >> > why did you introduce a new block scope? >> To separate the action block from the other code. "A Poor/lazy man's >> method". I've preferred to lay it out this way because it makes the code >> more compact and easy to read (as opposing to looking for a only-once used >> method in some other part of the file) and due to the confusing name of >> OutputAnalyzer type (which I need as a storage for output, not as some >> analyzer). Creating a method with OutputAnalyzer as a parameter would make >> this method's purpose even more confusing. >> > You can use printf rather than making a separate call to format. >> Good point, I've missed that method. I'll fix it and ask Igor to change the >> webrev. >> Regards, >> Evgeny. >>> On 2020-04-07 14:50, David Holmes wrote: >>> Hi Evgeny, >>> >>> On 7/04/2020 6:33 pm, Evgeny Nikitin wrote: >>>> Hi David, >>>> >>>>> I'm not at all sure this is generally what we want for every single test >>>>> that uses ProcessTools! But I'm willing it to see it trialed. >>>> >>>> Well, it's mostly used for test VM runs. In runs I performed (artificial >>>> "failures" included) the amounts of output were very small. >>>> >>>>> Please run full tier testing at least to tier 6 and ideally beyond before >>>>> pushing this. There are potential implications for temporary (and more >>>>> permanent) disk usage as well as additional time needed to write files >>>>> out to disk. (Hopefully these are generally small enough that this >>>>> doesn't make a noticeable difference.) >>>> Done, thanks for suggestion. The additional runs showed no problems. >>> >>> Good to know - thanks. >>> >>>> I've provided Igor with a slightly modified version (Added notification >>>> about the output file into the main test's log). Please review: >>>> >>>> http://cr.openjdk.java.net/~iignatyev/enikitin/8174768/webrev.01/ >>> >>> A couple of minor nits: >>> >>> 397 { >>> >>> why did you introduce a new block scope? >>> >>> 404 System.out.println(String.format( >>> >>> You can use printf rather than making a separate call to format. >>> >>> No need to see any update if you chose to make it. >>> >>> Thanks, >>> David >>> ----- >>> >>>> Best Regards, >>>> >>>> Evgeny. >>>> >>>> On 2020-04-02 02:07, David Holmes wrote: >>>>> Thanks for sharing this Igor! >>>>> >>>>> I'm not at all sure this is generally what we want for every single test >>>>> that uses ProcessTools! But I'm willing it to see it trialed. >>>>> >>>>> Evgeny: Please run full tier testing at least to tier 6 and ideally >>>>> beyond before pushing this. There are potential implications for >>>>> temporary (and more permanent) disk usage as well as additional time >>>>> needed to write files out to disk. (Hopefully these are generally small >>>>> enough that this doesn't make a noticeable difference.) >>>>> >>>>> Thanks, >>>>> David >>>>> >>>>> On 2/04/2020 5:13 am, Igor Ignatyev wrote: >>>>>> Hi Evgeny, >>>>>> >>>>>> (widening the audience, given this affects not just hotspot compiler, >>>>>> but hotspot tests as well as core libs tests in general) >>>>>> >>>>>> overall that looks good to me. one suggestion, for the ease of failure >>>>>> analysis it might be worth to print out the names of created files, >>>>>> although this might potentially clutter the output, I don't think it'll >>>>>> be a problem given we already print out things like 'Gathering output >>>>>> for process ...' , 'Waiting for completion...' in LazyOutputBuffer. >>>>>> >>>>>>> The change has been tested via a mach5 test runs (jdk-tier1 through 4) >>>>>>> on the 4 common platforms (linux-x64, windows-x64, macosx-x64, sparcv9). >>>>>> this doesn't include any of hotspot tiers, could you please also run >>>>>> hs-tier1--4? >>>>>> // you can use tierN jobs which include both jdk and hs parts. >>>>>> >>>>>> Thanks, >>>>>> -- Igor >>>>>> >>>>>>> On Mar 30, 2020, at 3:55 AM, Evgeny Nikitin <evgeny.niki...@oracle.com> >>>>>>> wrote: >>>>>>> >>>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> >>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8174768 >>>>>>> >>>>>>> Webrev: >>>>>>> http://cr.openjdk.java.net/~iignatyev/enikitin/8174768/webrev.00/ >>>>>>> >>>>>>> >>>>>>> The bug had been created as a request to simplify investigation for >>>>>>> compiler control tests failures. >>>>>>> I found the functionality pretty generic and useful and made >>>>>>> ProcessTools dump output as well as some diagnostic information for >>>>>>> every executed process into a separate file. >>>>>>> The diagnostic information contains cmdline, exit code, stdout and >>>>>>> stderr. The output files are named like 'pid-<PID>-output.log'. >>>>>>> >>>>>>> The change has been tested via a mach5 test runs (jdk-tier1 through 4) >>>>>>> on the 4 common platforms (linux-x64, windows-x64, macosx-x64, sparcv9). >>>>>>> >>>>>>> Please review, >>>>>>> /Evgeny Nikitin. >>>>>>
