From bb79e908bee715cbcc0d2dd47ac60f11908f4088 Mon Sep 17 00:00:00 2001 From: "M. Seledtsov" Date: Tue, 24 Jan 2023 19:51:54 -0800 Subject: [PATCH 01/16] Add diagnostic logging to at-requires VMProps --- test/jtreg-ext/requires/VMProps.java | 41 +++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/test/jtreg-ext/requires/VMProps.java b/test/jtreg-ext/requires/VMProps.java index 8d16d3a96607b..9f814d68b7e81 100644 --- a/test/jtreg-ext/requires/VMProps.java +++ b/test/jtreg-ext/requires/VMProps.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -27,6 +27,7 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.charset.Charset; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -87,6 +88,7 @@ public void put(String key, Supplier s) { */ @Override public Map call() { + log("Entering call()"); SafeMap map = new SafeMap(); map.put("vm.flavor", this::vmFlavor); map.put("vm.compMode", this::vmCompMode); @@ -127,6 +129,7 @@ public Map call() { vmOptFinalFlags(map); dump(map.map); + log("Leaving call()"); return map.map; } @@ -473,6 +476,8 @@ protected String isCompiler2Enabled() { * @return true if docker is supported in a given environment */ protected String dockerSupport() { + log("Entering dockerSupport()"); + boolean isSupported = false; if (Platform.isLinux()) { // currently docker testing is only supported for Linux, @@ -491,6 +496,8 @@ protected String dockerSupport() { } } + log("dockerSupport(): isSupported = " + isSupported); + if (isSupported) { try { isSupported = checkDockerSupport(); @@ -622,6 +629,38 @@ protected static void dump(Map map) { } } + /** + * Logs diagnostic message. + * + * @param msg + */ + protected static void log(String msg) { + logToFile(msg); + } + + /** + * Logs diagnostic message into a file. + * Use a property -Djtreg.ext.at.requires.logfile to specify file name. + * E.g.: jtreg -Djtreg.ext.at.requires.logfile="/tmp/jtreg-at-requires.log". + * If a property is not specified method returns w/o any action. + * + * @param msg + */ + protected static void logToFile(String msg) { + String fileName = System.getProperty("jtreg.ext.at.requires.logfile"); + if (fileName == null) { + return; + } + + try { + Files.writeString(Paths.get(fileName), msg + "\n", Charset.forName("ISO-8859-1"), + StandardOpenOption.APPEND, StandardOpenOption.CREATE); + } catch (IOException e) { + throw new RuntimeException("Failed to log into '" + + fileName + "'", e); + } + } + /** * This method is for the testing purpose only. * From 632432f380efc5306ee86853fc7e6900e6ac32a4 Mon Sep 17 00:00:00 2001 From: "M. Seledtsov" Date: Wed, 25 Jan 2023 15:30:28 -0800 Subject: [PATCH 02/16] Allow both absolute and relative path for log file --- test/jtreg-ext/requires/VMProps.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/jtreg-ext/requires/VMProps.java b/test/jtreg-ext/requires/VMProps.java index 9f814d68b7e81..d800bb2d7f734 100644 --- a/test/jtreg-ext/requires/VMProps.java +++ b/test/jtreg-ext/requires/VMProps.java @@ -27,6 +27,7 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; +import java.io.File; import java.nio.charset.Charset; import java.nio.file.Files; import java.nio.file.Path; @@ -406,6 +407,7 @@ protected String vmRTMCPU() { * @return true if CDS is supported by the VM to be tested. */ protected String vmCDS() { + log("vmCDS()"); return "" + WB.isCDSIncluded(); } @@ -652,6 +654,13 @@ protected static void logToFile(String msg) { return; } + // If path starts with the separator it will be used as is, as an absolute path. + // Otherwise it will be treated as a path relative to the jtreg current working directory. + String currentWorkDir = "."; + if(!fileName.startsWith(File.separator)) { + fileName = currentWorkDir + File.separator + fileName; + } + try { Files.writeString(Paths.get(fileName), msg + "\n", Charset.forName("ISO-8859-1"), StandardOpenOption.APPEND, StandardOpenOption.CREATE); From 5deb681b6cef67ac5234587eeec7e82bd64dd620 Mon Sep 17 00:00:00 2001 From: "M. Seledtsov" Date: Wed, 25 Jan 2023 18:24:59 -0800 Subject: [PATCH 03/16] More logging for container methods --- test/jtreg-ext/requires/VMProps.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/jtreg-ext/requires/VMProps.java b/test/jtreg-ext/requires/VMProps.java index d800bb2d7f734..18f310f0b98a1 100644 --- a/test/jtreg-ext/requires/VMProps.java +++ b/test/jtreg-ext/requires/VMProps.java @@ -498,7 +498,7 @@ protected String dockerSupport() { } } - log("dockerSupport(): isSupported = " + isSupported); + log("dockerSupport(): platform check: isSupported = " + isSupported); if (isSupported) { try { @@ -508,15 +508,19 @@ protected String dockerSupport() { } } + log("dockerSupport(): returning isSupported = " + isSupported); return "" + isSupported; } private boolean checkDockerSupport() throws IOException, InterruptedException { + log("checkDockerSupport(): entering"); ProcessBuilder pb = new ProcessBuilder(Container.ENGINE_COMMAND, "ps"); Process p = pb.start(); p.waitFor(10, TimeUnit.SECONDS); + int exitValue = p.exitValue(); + log("checkDockerSupport(): exitValue = " + exitValue); - return (p.exitValue() == 0); + return (exitValue == 0); } /** From 35afd7c7142d2a76e8e6c22938fd2ce306be248a Mon Sep 17 00:00:00 2001 From: "M. Seledtsov" Date: Wed, 25 Jan 2023 19:38:42 -0800 Subject: [PATCH 04/16] Draft: redirect container ps child output to log file --- test/jtreg-ext/requires/VMProps.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/jtreg-ext/requires/VMProps.java b/test/jtreg-ext/requires/VMProps.java index 18f310f0b98a1..00a7c41935828 100644 --- a/test/jtreg-ext/requires/VMProps.java +++ b/test/jtreg-ext/requires/VMProps.java @@ -515,9 +515,17 @@ protected String dockerSupport() { private boolean checkDockerSupport() throws IOException, InterruptedException { log("checkDockerSupport(): entering"); ProcessBuilder pb = new ProcessBuilder(Container.ENGINE_COMMAND, "ps"); + + // TODO: use property for file path if defined, plus timestamp + File out = new File("./container-ps-stdout.log"); + pb.redirectOutput(out); + File err = new File("./container-ps-stderr.log"); + pb.redirectError(err); Process p = pb.start(); p.waitFor(10, TimeUnit.SECONDS); int exitValue = p.exitValue(); + + // TODO: log stdout and stderr file names log("checkDockerSupport(): exitValue = " + exitValue); return (exitValue == 0); @@ -674,6 +682,7 @@ protected static void logToFile(String msg) { } } + /** * This method is for the testing purpose only. * From bcf46910cb08d6aec5defba5794130e0ee2e6a20 Mon Sep 17 00:00:00 2001 From: "M. Seledtsov" Date: Thu, 26 Jan 2023 13:17:10 -0800 Subject: [PATCH 05/16] Simplification and refactoring --- test/jtreg-ext/requires/VMProps.java | 42 ++++++++++++++++------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/test/jtreg-ext/requires/VMProps.java b/test/jtreg-ext/requires/VMProps.java index 00a7c41935828..c21a5320ce52c 100644 --- a/test/jtreg-ext/requires/VMProps.java +++ b/test/jtreg-ext/requires/VMProps.java @@ -33,6 +33,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.StandardOpenOption; +import java.time.Instant; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -512,21 +513,32 @@ protected String dockerSupport() { return "" + isSupported; } + private String redirectOutputToLogFile(ProcessBuilder pb, String fileNameBase) { + if (!Boolean.getBoolean("jtreg.log.vmprops")) { + return ""; + } + + String fileName = "./" + fileNameBase + Instant.now().toString() + ".log"; + pb.redirectOutput(new File(fileName)); + return fileName; + } + private boolean checkDockerSupport() throws IOException, InterruptedException { log("checkDockerSupport(): entering"); ProcessBuilder pb = new ProcessBuilder(Container.ENGINE_COMMAND, "ps"); // TODO: use property for file path if defined, plus timestamp - File out = new File("./container-ps-stdout.log"); - pb.redirectOutput(out); - File err = new File("./container-ps-stderr.log"); - pb.redirectError(err); + String stdoutFn = redirectOutputToLogFile(pb, "container-ps-stdout"); + String stderrFn = redirectOutputToLogFile(pb, "container-ps-stderr"); Process p = pb.start(); p.waitFor(10, TimeUnit.SECONDS); int exitValue = p.exitValue(); - // TODO: log stdout and stderr file names log("checkDockerSupport(): exitValue = " + exitValue); + log(String.format("checkDockerSupport(): child process stdout for pid %s was logged into %s", + p.pid(), stdoutFn)); + log(String.format("checkDockerSupport(): child process stderr for pid %s was logged into %s", + p.pid(), stderrFn)); return (exitValue == 0); } @@ -652,33 +664,27 @@ protected static void log(String msg) { logToFile(msg); } + /** * Logs diagnostic message into a file. - * Use a property -Djtreg.ext.at.requires.logfile to specify file name. - * E.g.: jtreg -Djtreg.ext.at.requires.logfile="/tmp/jtreg-at-requires.log". + * Use a property -Djtreg.log.vmprops to turn on vmprops logging. + * E.g.: jtreg -Djtreg.log.vmprops=true * If a property is not specified method returns w/o any action. + * Log will be writtent into current jtreg's workdir. * * @param msg */ protected static void logToFile(String msg) { - String fileName = System.getProperty("jtreg.ext.at.requires.logfile"); - if (fileName == null) { + if (!Boolean.getBoolean("jtreg.log.vmprops")) { return; } - // If path starts with the separator it will be used as is, as an absolute path. - // Otherwise it will be treated as a path relative to the jtreg current working directory. - String currentWorkDir = "."; - if(!fileName.startsWith(File.separator)) { - fileName = currentWorkDir + File.separator + fileName; - } - + String fileName = "./jtreg-vm-props.log"; try { Files.writeString(Paths.get(fileName), msg + "\n", Charset.forName("ISO-8859-1"), StandardOpenOption.APPEND, StandardOpenOption.CREATE); } catch (IOException e) { - throw new RuntimeException("Failed to log into '" - + fileName + "'", e); + throw new RuntimeException("Failed to log into '" + fileName + "'", e); } } From da9c27c6aaabdf633d62b4b41d8d11374e396c9c Mon Sep 17 00:00:00 2001 From: "M. Seledtsov" Date: Mon, 30 Jan 2023 10:59:33 -0800 Subject: [PATCH 06/16] Fixed issues with process out/err redirect --- test/jtreg-ext/requires/VMProps.java | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/test/jtreg-ext/requires/VMProps.java b/test/jtreg-ext/requires/VMProps.java index c21a5320ce52c..5fa3cb79ed2e5 100644 --- a/test/jtreg-ext/requires/VMProps.java +++ b/test/jtreg-ext/requires/VMProps.java @@ -513,33 +513,31 @@ protected String dockerSupport() { return "" + isSupported; } - private String redirectOutputToLogFile(ProcessBuilder pb, String fileNameBase) { + private void redirectOutputToLogFile(ProcessBuilder pb, String fileNameBase) { if (!Boolean.getBoolean("jtreg.log.vmprops")) { - return ""; + return; } - String fileName = "./" + fileNameBase + Instant.now().toString() + ".log"; - pb.redirectOutput(new File(fileName)); - return fileName; + String timeStamp = Instant.now().toString().replace(":", "-").replace(".", "-"); + + String stdoutFileName = String.format("./%s-stdout--%s.log", fileNameBase, timeStamp); + pb.redirectOutput(new File(stdoutFileName)); + log("checkDockerSupport(): child process stdout redirected to" + stdoutFileName); + + String stderrFileName = String.format("./%s-stderr--%s.log", fileNameBase, timeStamp); + pb.redirectError(new File(stderrFileName)); + log("checkDockerSupport(): child process stderr redirected to" + stderrFileName); } private boolean checkDockerSupport() throws IOException, InterruptedException { log("checkDockerSupport(): entering"); ProcessBuilder pb = new ProcessBuilder(Container.ENGINE_COMMAND, "ps"); - - // TODO: use property for file path if defined, plus timestamp - String stdoutFn = redirectOutputToLogFile(pb, "container-ps-stdout"); - String stderrFn = redirectOutputToLogFile(pb, "container-ps-stderr"); + redirectOutputToLogFile(pb, "container-ps-stdout"); Process p = pb.start(); p.waitFor(10, TimeUnit.SECONDS); int exitValue = p.exitValue(); - log("checkDockerSupport(): exitValue = " + exitValue); - log(String.format("checkDockerSupport(): child process stdout for pid %s was logged into %s", - p.pid(), stdoutFn)); - log(String.format("checkDockerSupport(): child process stderr for pid %s was logged into %s", - p.pid(), stderrFn)); - + log(String.format("checkDockerSupport(): exitValue = %s, pid = %s", exitValue, p.pid())); return (exitValue == 0); } From 562168625db63f0e9345ee4be2ce58f9caffae08 Mon Sep 17 00:00:00 2001 From: "M. Seledtsov" Date: Mon, 30 Jan 2023 11:03:45 -0800 Subject: [PATCH 07/16] Log to stderr as per discussion --- test/jtreg-ext/requires/VMProps.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jtreg-ext/requires/VMProps.java b/test/jtreg-ext/requires/VMProps.java index 5fa3cb79ed2e5..2d00098568d2a 100644 --- a/test/jtreg-ext/requires/VMProps.java +++ b/test/jtreg-ext/requires/VMProps.java @@ -659,6 +659,7 @@ protected static void dump(Map map) { * @param msg */ protected static void log(String msg) { + System.err.println("VMProps: " + msg); logToFile(msg); } @@ -676,7 +677,6 @@ protected static void logToFile(String msg) { if (!Boolean.getBoolean("jtreg.log.vmprops")) { return; } - String fileName = "./jtreg-vm-props.log"; try { Files.writeString(Paths.get(fileName), msg + "\n", Charset.forName("ISO-8859-1"), From 7851e5d4abc658e76c95e6e285169f020eb1a72f Mon Sep 17 00:00:00 2001 From: "M. Seledtsov" Date: Mon, 30 Jan 2023 15:43:34 -0800 Subject: [PATCH 08/16] Minor fixes in printed message --- test/jtreg-ext/requires/VMProps.java | 39 +++++----------------------- 1 file changed, 6 insertions(+), 33 deletions(-) diff --git a/test/jtreg-ext/requires/VMProps.java b/test/jtreg-ext/requires/VMProps.java index 2d00098568d2a..170c00fcc55d2 100644 --- a/test/jtreg-ext/requires/VMProps.java +++ b/test/jtreg-ext/requires/VMProps.java @@ -513,26 +513,22 @@ protected String dockerSupport() { return "" + isSupported; } - private void redirectOutputToLogFile(ProcessBuilder pb, String fileNameBase) { - if (!Boolean.getBoolean("jtreg.log.vmprops")) { - return; - } - + private void redirectOutputToLogFile(String msg, ProcessBuilder pb, String fileNameBase) { String timeStamp = Instant.now().toString().replace(":", "-").replace(".", "-"); String stdoutFileName = String.format("./%s-stdout--%s.log", fileNameBase, timeStamp); pb.redirectOutput(new File(stdoutFileName)); - log("checkDockerSupport(): child process stdout redirected to" + stdoutFileName); + log(msg + ": child process stdout redirected to " + stdoutFileName); String stderrFileName = String.format("./%s-stderr--%s.log", fileNameBase, timeStamp); pb.redirectError(new File(stderrFileName)); - log("checkDockerSupport(): child process stderr redirected to" + stderrFileName); + log(msg + ": child process stderr redirected to " + stderrFileName); } private boolean checkDockerSupport() throws IOException, InterruptedException { log("checkDockerSupport(): entering"); ProcessBuilder pb = new ProcessBuilder(Container.ENGINE_COMMAND, "ps"); - redirectOutputToLogFile(pb, "container-ps-stdout"); + redirectOutputToLogFile("checkDockerSupport(): ps", pb, "container-ps"); Process p = pb.start(); p.waitFor(10, TimeUnit.SECONDS); int exitValue = p.exitValue(); @@ -659,34 +655,11 @@ protected static void dump(Map map) { * @param msg */ protected static void log(String msg) { + // By jtreg design stderr produced here will be visible + // in the output of a parent process. System.err.println("VMProps: " + msg); - logToFile(msg); } - - /** - * Logs diagnostic message into a file. - * Use a property -Djtreg.log.vmprops to turn on vmprops logging. - * E.g.: jtreg -Djtreg.log.vmprops=true - * If a property is not specified method returns w/o any action. - * Log will be writtent into current jtreg's workdir. - * - * @param msg - */ - protected static void logToFile(String msg) { - if (!Boolean.getBoolean("jtreg.log.vmprops")) { - return; - } - String fileName = "./jtreg-vm-props.log"; - try { - Files.writeString(Paths.get(fileName), msg + "\n", Charset.forName("ISO-8859-1"), - StandardOpenOption.APPEND, StandardOpenOption.CREATE); - } catch (IOException e) { - throw new RuntimeException("Failed to log into '" + fileName + "'", e); - } - } - - /** * This method is for the testing purpose only. * From 4465bf13bb57e5fc1d59813722af9ede8acf1d0a Mon Sep 17 00:00:00 2001 From: "M. Seledtsov" Date: Mon, 30 Jan 2023 16:10:22 -0800 Subject: [PATCH 09/16] Conditionally enable VMProps logging --- test/jtreg-ext/requires/VMProps.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/jtreg-ext/requires/VMProps.java b/test/jtreg-ext/requires/VMProps.java index 170c00fcc55d2..a32f88bcc430a 100644 --- a/test/jtreg-ext/requires/VMProps.java +++ b/test/jtreg-ext/requires/VMProps.java @@ -514,6 +514,9 @@ protected String dockerSupport() { } private void redirectOutputToLogFile(String msg, ProcessBuilder pb, String fileNameBase) { + if (!Boolean.getBoolean("jtreg.log.vmprops")) { + return; + } String timeStamp = Instant.now().toString().replace(":", "-").replace(".", "-"); String stdoutFileName = String.format("./%s-stdout--%s.log", fileNameBase, timeStamp); @@ -655,6 +658,9 @@ protected static void dump(Map map) { * @param msg */ protected static void log(String msg) { + if (!Boolean.getBoolean("jtreg.log.vmprops")) { + return; + } // By jtreg design stderr produced here will be visible // in the output of a parent process. System.err.println("VMProps: " + msg); From 881a547ee5b1d59ef7c1fe62016ddc5c324200e9 Mon Sep 17 00:00:00 2001 From: "M. Seledtsov" Date: Mon, 30 Jan 2023 16:13:47 -0800 Subject: [PATCH 10/16] Comment about logging to stdout --- test/jtreg-ext/requires/VMProps.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/jtreg-ext/requires/VMProps.java b/test/jtreg-ext/requires/VMProps.java index a32f88bcc430a..0d63c8cb14900 100644 --- a/test/jtreg-ext/requires/VMProps.java +++ b/test/jtreg-ext/requires/VMProps.java @@ -663,6 +663,8 @@ protected static void log(String msg) { } // By jtreg design stderr produced here will be visible // in the output of a parent process. + // Avoid logging to stdout as it will be filtered out by the jtreg + // main process. System.err.println("VMProps: " + msg); } From 02d6833afdfd380a34c8b97fe69b9df6730d7b60 Mon Sep 17 00:00:00 2001 From: "M. Seledtsov" Date: Tue, 31 Jan 2023 18:31:27 -0800 Subject: [PATCH 11/16] Updated comment as per review feedback --- test/jtreg-ext/requires/VMProps.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/jtreg-ext/requires/VMProps.java b/test/jtreg-ext/requires/VMProps.java index 0d63c8cb14900..3ba2c795a1200 100644 --- a/test/jtreg-ext/requires/VMProps.java +++ b/test/jtreg-ext/requires/VMProps.java @@ -662,9 +662,9 @@ protected static void log(String msg) { return; } // By jtreg design stderr produced here will be visible - // in the output of a parent process. - // Avoid logging to stdout as it will be filtered out by the jtreg - // main process. + // in the output of a parent process. Note: stdout should not be used + // for logging as jtreg parses that output directly and only echoes it + // in the event of a failure. System.err.println("VMProps: " + msg); } From b1aa1c4b9a93fa475e4772fb02039c39211a9f5b Mon Sep 17 00:00:00 2001 From: "M. Seledtsov" Date: Tue, 31 Jan 2023 19:43:21 -0800 Subject: [PATCH 12/16] Review feedback: printing stdout and stderr files to log() --- test/jtreg-ext/requires/VMProps.java | 37 +++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/test/jtreg-ext/requires/VMProps.java b/test/jtreg-ext/requires/VMProps.java index 3ba2c795a1200..ba7d436adba2b 100644 --- a/test/jtreg-ext/requires/VMProps.java +++ b/test/jtreg-ext/requires/VMProps.java @@ -513,9 +513,10 @@ protected String dockerSupport() { return "" + isSupported; } - private void redirectOutputToLogFile(String msg, ProcessBuilder pb, String fileNameBase) { + // Returns comma-separated file names for stdout and stderr. + private String redirectOutputToLogFile(String msg, ProcessBuilder pb, String fileNameBase) { if (!Boolean.getBoolean("jtreg.log.vmprops")) { - return; + return ""; } String timeStamp = Instant.now().toString().replace(":", "-").replace(".", "-"); @@ -526,17 +527,45 @@ private void redirectOutputToLogFile(String msg, ProcessBuilder pb, String fileN String stderrFileName = String.format("./%s-stderr--%s.log", fileNameBase, timeStamp); pb.redirectError(new File(stderrFileName)); log(msg + ": child process stderr redirected to " + stderrFileName); - } + + return stdoutFileName + "," + stderrFileName; + } + + private void printLogfileContent(String logFileNames) { + if (logFileNames.isEmpty()) { + return; + } + + log("------------- stdout: "); + try { + Files.lines(Path.of(logFileNames.split(",")[0])) + .forEach(line -> log(line)); + } catch (IOException ie) { + log("Exception while reading stdout file: " + ie); + } + log("------------- "); + + log("------------- stderr: "); + try { + Files.lines(Path.of(logFileNames.split(",")[1])) + .forEach(line -> log(line)); + } catch (IOException ie) { + log("Exception while reading stderr file: " + ie); + } + log("------------- "); + } private boolean checkDockerSupport() throws IOException, InterruptedException { log("checkDockerSupport(): entering"); ProcessBuilder pb = new ProcessBuilder(Container.ENGINE_COMMAND, "ps"); - redirectOutputToLogFile("checkDockerSupport(): ps", pb, "container-ps"); + String logFileNames = redirectOutputToLogFile("checkDockerSupport(): ps", + pb, "container-ps"); Process p = pb.start(); p.waitFor(10, TimeUnit.SECONDS); int exitValue = p.exitValue(); log(String.format("checkDockerSupport(): exitValue = %s, pid = %s", exitValue, p.pid())); + printLogfileContent(logFileNames); return (exitValue == 0); } From ceb0ed65744982d065edfd5efa1a6bef0df93e79 Mon Sep 17 00:00:00 2001 From: "M. Seledtsov" Date: Wed, 1 Feb 2023 13:17:11 -0800 Subject: [PATCH 13/16] Addressing review feedback --- test/jtreg-ext/requires/VMProps.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/jtreg-ext/requires/VMProps.java b/test/jtreg-ext/requires/VMProps.java index ba7d436adba2b..17f00c80512aa 100644 --- a/test/jtreg-ext/requires/VMProps.java +++ b/test/jtreg-ext/requires/VMProps.java @@ -408,7 +408,6 @@ protected String vmRTMCPU() { * @return true if CDS is supported by the VM to be tested. */ protected String vmCDS() { - log("vmCDS()"); return "" + WB.isCDSIncluded(); } @@ -539,7 +538,7 @@ private void printLogfileContent(String logFileNames) { log("------------- stdout: "); try { Files.lines(Path.of(logFileNames.split(",")[0])) - .forEach(line -> log(line)); + .forEach(line -> log(line)); } catch (IOException ie) { log("Exception while reading stdout file: " + ie); } @@ -565,7 +564,10 @@ private boolean checkDockerSupport() throws IOException, InterruptedException { int exitValue = p.exitValue(); log(String.format("checkDockerSupport(): exitValue = %s, pid = %s", exitValue, p.pid())); - printLogfileContent(logFileNames); + if (exitValue != 0) { + printLogfileContent(logFileNames); + } + return (exitValue == 0); } From 1eef0f6f51c8e532f44fb09ebb55ae5e6ac9d7f7 Mon Sep 17 00:00:00 2001 From: "M. Seledtsov" Date: Wed, 1 Feb 2023 15:53:57 -0800 Subject: [PATCH 14/16] Removed logging condition as per review feedback --- test/jtreg-ext/requires/VMProps.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/jtreg-ext/requires/VMProps.java b/test/jtreg-ext/requires/VMProps.java index 17f00c80512aa..a7722b19751b3 100644 --- a/test/jtreg-ext/requires/VMProps.java +++ b/test/jtreg-ext/requires/VMProps.java @@ -514,9 +514,6 @@ protected String dockerSupport() { // Returns comma-separated file names for stdout and stderr. private String redirectOutputToLogFile(String msg, ProcessBuilder pb, String fileNameBase) { - if (!Boolean.getBoolean("jtreg.log.vmprops")) { - return ""; - } String timeStamp = Instant.now().toString().replace(":", "-").replace(".", "-"); String stdoutFileName = String.format("./%s-stdout--%s.log", fileNameBase, timeStamp); @@ -689,9 +686,6 @@ protected static void dump(Map map) { * @param msg */ protected static void log(String msg) { - if (!Boolean.getBoolean("jtreg.log.vmprops")) { - return; - } // By jtreg design stderr produced here will be visible // in the output of a parent process. Note: stdout should not be used // for logging as jtreg parses that output directly and only echoes it From b85636da7e1701eb587cb69610bd06ea6bbc588d Mon Sep 17 00:00:00 2001 From: "M. Seledtsov" Date: Wed, 8 Feb 2023 15:13:33 -0800 Subject: [PATCH 15/16] Re-added conditional logging per review discussion --- test/jtreg-ext/requires/VMProps.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/jtreg-ext/requires/VMProps.java b/test/jtreg-ext/requires/VMProps.java index a7722b19751b3..b788985f03df1 100644 --- a/test/jtreg-ext/requires/VMProps.java +++ b/test/jtreg-ext/requires/VMProps.java @@ -514,6 +514,9 @@ protected String dockerSupport() { // Returns comma-separated file names for stdout and stderr. private String redirectOutputToLogFile(String msg, ProcessBuilder pb, String fileNameBase) { + if (!Boolean.getBoolean("jtreg.log.vmprops")) { + return ""; + } String timeStamp = Instant.now().toString().replace(":", "-").replace(".", "-"); String stdoutFileName = String.format("./%s-stdout--%s.log", fileNameBase, timeStamp); @@ -686,6 +689,10 @@ protected static void dump(Map map) { * @param msg */ protected static void log(String msg) { + if (!Boolean.getBoolean("jtreg.log.vmprops")) { + return; + } + // By jtreg design stderr produced here will be visible // in the output of a parent process. Note: stdout should not be used // for logging as jtreg parses that output directly and only echoes it From e1890eaaac45051fe773abc7dadc6bdd97989f84 Mon Sep 17 00:00:00 2001 From: "M. Seledtsov" Date: Tue, 14 Feb 2023 16:31:51 -0800 Subject: [PATCH 16/16] Addressed recent review feedback from Leonid --- test/jtreg-ext/requires/VMProps.java | 77 ++++++++++++++++------------ 1 file changed, 43 insertions(+), 34 deletions(-) diff --git a/test/jtreg-ext/requires/VMProps.java b/test/jtreg-ext/requires/VMProps.java index b788985f03df1..fe1bc8d072012 100644 --- a/test/jtreg-ext/requires/VMProps.java +++ b/test/jtreg-ext/requires/VMProps.java @@ -512,52 +512,44 @@ protected String dockerSupport() { return "" + isSupported; } - // Returns comma-separated file names for stdout and stderr. - private String redirectOutputToLogFile(String msg, ProcessBuilder pb, String fileNameBase) { - if (!Boolean.getBoolean("jtreg.log.vmprops")) { - return ""; - } + // Configures process builder to redirect process stdout and stderr to a file. + // Returns file names for stdout and stderr. + private Map redirectOutputToLogFile(String msg, ProcessBuilder pb, String fileNameBase) { + Map result = new HashMap<>(); String timeStamp = Instant.now().toString().replace(":", "-").replace(".", "-"); String stdoutFileName = String.format("./%s-stdout--%s.log", fileNameBase, timeStamp); pb.redirectOutput(new File(stdoutFileName)); log(msg + ": child process stdout redirected to " + stdoutFileName); + result.put("stdout", stdoutFileName); String stderrFileName = String.format("./%s-stderr--%s.log", fileNameBase, timeStamp); pb.redirectError(new File(stderrFileName)); log(msg + ": child process stderr redirected to " + stderrFileName); + result.put("stderr", stderrFileName); - return stdoutFileName + "," + stderrFileName; + return result; } - private void printLogfileContent(String logFileNames) { - if (logFileNames.isEmpty()) { - return; - } - - log("------------- stdout: "); - try { - Files.lines(Path.of(logFileNames.split(",")[0])) - .forEach(line -> log(line)); - } catch (IOException ie) { - log("Exception while reading stdout file: " + ie); - } - log("------------- "); - - log("------------- stderr: "); - try { - Files.lines(Path.of(logFileNames.split(",")[1])) - .forEach(line -> log(line)); - } catch (IOException ie) { - log("Exception while reading stderr file: " + ie); - } - log("------------- "); + private void printLogfileContent(Map logFileNames) { + logFileNames.entrySet().stream() + .forEach(entry -> + { + log("------------- " + entry.getKey()); + try { + Files.lines(Path.of(entry.getValue())) + .forEach(line -> log(line)); + } catch (IOException ie) { + log("Exception while reading file: " + ie); + } + log("-------------"); + }); } private boolean checkDockerSupport() throws IOException, InterruptedException { log("checkDockerSupport(): entering"); ProcessBuilder pb = new ProcessBuilder(Container.ENGINE_COMMAND, "ps"); - String logFileNames = redirectOutputToLogFile("checkDockerSupport(): ps", + Map logFileNames = redirectOutputToLogFile("checkDockerSupport(): ps", pb, "container-ps"); Process p = pb.start(); p.waitFor(10, TimeUnit.SECONDS); @@ -684,20 +676,37 @@ protected static void dump(Map map) { } /** - * Logs diagnostic message. + * Log diagnostic message. * * @param msg */ protected static void log(String msg) { - if (!Boolean.getBoolean("jtreg.log.vmprops")) { - return; - } + // Always log to a file. + logToFile(msg); + // Also log to stderr; guarded by property to avoid excessive verbosity. // By jtreg design stderr produced here will be visible // in the output of a parent process. Note: stdout should not be used // for logging as jtreg parses that output directly and only echoes it // in the event of a failure. - System.err.println("VMProps: " + msg); + if (Boolean.getBoolean("jtreg.log.vmprops")) { + System.err.println("VMProps: " + msg); + } + } + + /** + * Log diagnostic message to a file. + * + * @param msg + */ + protected static void logToFile(String msg) { + String fileName = "./vmprops.log"; + try { + Files.writeString(Paths.get(fileName), msg + "\n", Charset.forName("ISO-8859-1"), + StandardOpenOption.APPEND, StandardOpenOption.CREATE); + } catch (IOException e) { + throw new RuntimeException("Failed to log into '" + fileName + "'", e); + } } /**