RFR: 8244993: Revert changes to OutputAnalyzer stderrShouldBeEmptyIgnoreVMWarnings() that allow version strings

[Chris Plummer]

Hi Daniil,

Looks good.

thanks,

Chris

On 5/26/20 10:46 AM, Daniil Titov wrote:
> Hi Chris and David,
>
> Please review a new version of the fix [1] with the changes Chris suggested.
>
> [1] Webrev:  http://cr.openjdk.java.net/~dtitov/8244993/webrev.02
> [2] Jira issue: https://bugs.openjdk.java.net/browse/JDK-8244993
>
> Thank you,
> Daniil
>
> On 5/22/20, 11:50 AM, "Chris Plummer" <chris.plummer at oracle.com> wrote:
>
>      Hi Daniil,
>
>      There is one reference to "jvmwarningmsg" that occurs before it is
>      declared while all the rest all come after. It probably would make sense
>      to move its declaration up near the top of the file.
>
>         92     private static void matchListedProcesses(OutputAnalyzer output) {
>         93         output.stdoutShouldMatchByLine(JCMD_LIST_REGEX)
>         94                 .stderrShouldBeEmptyIgnoreVMWarnings();
>         95     }
>
>      We probably use this coding pattern all over the place, but I think it
>      just leads to less readable code. Any reason not to use:
>
>         92     private static void matchListedProcesses(OutputAnalyzer output) {
>         93         output.stdoutShouldMatchByLine(JCMD_LIST_REGEX);
>         94         output.stderrShouldBeEmptyIgnoreVMWarnings();
>         95     }
>
>      I just don't see the point of the chaining, and don't understand why all
>      these OutputAnalyzer methods return the "this" object in the first
>      place. Maybe I'm missing something. I guess maybe there are cases where
>      the OutputAnalyzer object is not already in a local variable, adding
>      some value to the chaining, but that's not the case here, and I think if
>      it were the case it would be more readable just to stick the
>      OutputAnalyzer object in a local. There one other case of this:
>
>        154     private static void matchPerfCounters(OutputAnalyzer output) {
>        155 output.stdoutShouldMatchByLine(PERF_COUNTER_REGEX,null,
>        156                 PERF_COUNTER_REGEX)
>        157                 .stderrShouldBeEmptyIgnoreVMWarnings();
>        158     }
>
>      I think you can also add spaces after the commas, and probably make the
>      first stdoutShouldMatchByLine() one line.
>
>      thanks,
>
>      Chris
>
>      On 5/21/20 10:06 PM, Daniil Titov wrote:
>      > Please review a webrev [1] that reverts the changes done in jdk.test.lib.process.OutputAnalyzer in [3].
>      >
>      > Change [3] modified OutputAnalyzer stderrShouldBeEmptyIgnoreVMWarnings() methods to ignore also VM version strings . The current webrev [1] reverts this change and instead makes the tests that expects an empty stderr from launched j-* tools to filter out '-showversion' from test options when forwarding test VM option to j*-tools.
>      >
>      > Testing: Mach5 tier1-tier5 tests passed successfully. Tier6-tier7 tests are in progress.
>      >
>      > [1]  Webrev:  http://cr.openjdk.java.net/~dtitov/8244993/webrev.01
>      > [2] Jira issue: https://bugs.openjdk.java.net/browse/JDK-8244993
>      > [3] https://bugs.openjdk.java.net/browse/JDK-8242009
>      >
>      > Thank you,
>      > Daniil
>      >
>      >
>
>
>
>