On Sat, 15 Jun 2024 09:54:38 GMT, SendaoYan <s...@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
