RFR: 8334332: TestIOException.java fails if run by root [v2]

[Pavel Rappo]

On Sat, 15 Jun 2024 09:54:38 GMT, SendaoYan <syan at openjdk.org> wrote:

>> Hi all,
>>   Test `test/langtools/jdk/javadoc/doclet/testIOException/TestIOException.java` run fails with root user privileged. I think it's necessary to skip this testcase when user is root.
>>   Why run the jtreg test by root user? It's because during rpmbuild process for linux distribution of JDK, root user is the default user to build the openjdk, also is the default user to run the `make test-tier1`, this PR make this testcase more robustness.
>>   The change has been verified, only change the testcase, no risk.
>
> SendaoYan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   delete an extra whitespace

Come to think of it, there's already a mechanism in the test to skip a Windows machine if a directory cannot be made read-only. We could combine that mechanism with what you propose, simplifying and making code more robust at the same time.

Any time when we do `throw error`, it means that the test should be skipped. Cannot create a directory? Cannot make it read-only? Can write to a directory after having made it read-only? In any of these cases, our assumptions are invalid; the test should be skipped, not failed. So perhaps we could modify it like this?


diff --git a/test/langtools/jdk/javadoc/doclet/testIOException/TestIOException.java b/test/langtools/jdk/javadoc/doclet/testIOException/TestIOException.java
index 9127cdf86bb..fb01707511a 100644
--- a/test/langtools/jdk/javadoc/doclet/testIOException/TestIOException.java
+++ b/test/langtools/jdk/javadoc/doclet/testIOException/TestIOException.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2024, 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
@@ -25,7 +25,7 @@
  * @test
  * @bug 8164130
  * @summary test IOException handling
- * @library ../../lib
+ * @library ../../lib /test/lib
  * @modules jdk.javadoc/jdk.javadoc.internal.tool
  * @build javadoc.tester.*
  * @run main TestIOException
@@ -61,16 +61,13 @@ public static void main(String... args) throws Exception {
     public void testReadOnlyDirectory() {
         File outDir = new File("out1");
         if (!outDir.mkdir()) {
-            throw error(outDir, "Cannot create directory");
+            throw skip(outDir, "Cannot create directory");
         }
         if (!outDir.setReadOnly()) {
-            if (skip(outDir)) {
-                return;
-            }
-            throw error(outDir, "could not set directory read-only");
+            throw skip(outDir, "could not set directory read-only");
         }
         if (outDir.canWrite()) {
-            throw error(outDir, "directory is writable");
+            throw skip(outDir, "directory is writable");
         }
 
         try {
@@ -93,15 +90,15 @@ public void testReadOnlyDirectory() {
     public void testReadOnlyFile() throws Exception {
         File outDir = new File("out2");
         if (!outDir.mkdir()) {
-            throw error(outDir, "Cannot create directory");
+            throw skip(outDir, "Cannot create directory");
         }
         File index = new File(outDir, "index.html");
         try (FileWriter fw = new FileWriter(index)) { }
         if (!index.setReadOnly()) {
-            throw error(index, "could not set index read-only");
+            throw skip(index, "could not set index read-only");
         }
         if (index.canWrite()) {
-            throw error(index, "index is writable");
+            throw skip(index, "index is writable");
         }
 
         try {
@@ -139,16 +136,13 @@ public void testReadOnlySubdirectory() throws Exception {
         File outDir = new File("out3");
         File pkgOutDir = new File(outDir, "p");
         if (!pkgOutDir.mkdirs()) {
-            throw error(pkgOutDir, "Cannot create directory");
+            throw skip(pkgOutDir, "Cannot create directory");
         }
         if (!pkgOutDir.setReadOnly()) {
-            if (skip(pkgOutDir)) {
-                return;
-            }
-            throw error(pkgOutDir, "could not set directory read-only");
+            throw skip(pkgOutDir, "could not set directory read-only");
         }
         if (pkgOutDir.canWrite()) {
-            throw error(pkgOutDir, "directory is writable");
+            throw skip(pkgOutDir, "directory is writable");
         }
 
         // run javadoc and check results
@@ -192,16 +186,13 @@ public void testReadOnlyDocFilesDir() throws Exception {
         File pkgOutDir = new File(outDir, "p");
         File docFilesOutDir = new File(pkgOutDir, "doc-files");
         if (!docFilesOutDir.mkdirs()) {
-            throw error(docFilesOutDir, "Cannot create directory");
+            throw skip(docFilesOutDir, "Cannot create directory");
         }
         if (!docFilesOutDir.setReadOnly()) {
-            if (skip(docFilesOutDir)) {
-                return;
-            }
-            throw error(docFilesOutDir, "could not set directory read-only");
+            throw skip(docFilesOutDir, "could not set directory read-only");
         }
         if (docFilesOutDir.canWrite()) {
-            throw error(docFilesOutDir, "directory is writable");
+            throw skip(docFilesOutDir, "directory is writable");
         }
 
         try {
@@ -219,7 +210,8 @@ public void testReadOnlyDocFilesDir() throws Exception {
         }
     }
 
-    private Error error(File f, String message) {
+    private Error skip(File f, String message) {
+        out.print(System.getProperty("user.name"));
         out.println(f + ": " + message);
         showAllAttributes(f.toPath());
         throw new Error(f + ": " + message);
@@ -242,20 +234,5 @@ private void showAttributes(Path p, String attributes) {
             out.println("Error accessing attributes " + attributes + ": " + t);
         }
     }
-
-    private boolean skip(File dir) {
-        if (isWindows()) {
-            showAllAttributes(dir.toPath());
-            out.println("Windows: cannot set directory read only:" + dir);
-            out.println("TEST CASE SKIPPED");
-            return true;
-        } else {
-            return false;
-        }
-    }
-
-    private boolean isWindows() {
-        return System.getProperty("os.name").toLowerCase(Locale.US).startsWith("windows");
-    }
 }
 


This will work if people occasionally review the reasons why tests are skipped, not ignore skipped tests.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/19731#issuecomment-2173808185