Re: RFR: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows [v2]

2022-06-10 Thread Magnus Ihse Bursie
> The test  
> `test/jdk/java/util/ResourceBundle/Control/MissingResourceCauseTest.java` 
> verifies different failure modes of resource bundles. One of the failures is 
> that the runner class, `MissingResourceCauseTestRun.java`, creates a file 
> `UnreadableRB`, and runs `chmod 000` on it, to make it unreadable by the 
> test. Then MissingResourceCauseTest is called, and the `UnreadableRB` file is 
> removed.
> 
> This does not work reliably on Windows. On msys2, `chmod` is essentially a 
> no-op, so the file is not made unreadable, and hence the test fails. In my 
> personal cygwin test environment, the chmod command does have some effect, 
> but it is still not enough to make the file unreadable, and so the test fails.
> 
> The test was originally a shell script test that got converted to Java in 
> [JDK-4354216](https://bugs.openjdk.org/browse/JDK-8213127). The original 
> shell script code explicitly excluded Windows from testing. This was changed 
> in the rewrite, for reasons I cannot determine.
> 
> What suprises me, though, is the "how can this ever has worked???" factor. 
> Apparently the test passes on the current Cygwin setup on GHA. I have failed 
> to reproduce the working conditions that makes a file actually unreadable for 
> the owner on Windows, neither on my GHA test repo, nor locally. I've searched 
> the web to figure out how to properly set file permissions on Windows to make 
> the file unreadable, using Windows native tools, to no avail. I've even asked 
> a [Stack Overflow 
> question](https://stackoverflow.com/questions/72528318/what-file-permissions-make-a-file-unreadable-by-owner-in-windows);
>  which as of yet is still unanswered.
> 
> Since I feel I've spent far more time than reasonable trying to get this to 
> work properly, I suggest we instead skip the unreadable test on Windows. It 
> is clearly unstable and highly depending on the Windows environment, the test 
> was never originally supported or intended for Windows, and at the of the 
> day, testing file unreadability is not an important regression test for 
> [JDK-4354216](https://bugs.openjdk.org/browse/JDK-4354216).

Magnus Ihse Bursie has updated the pull request incrementally with one 
additional commit since the last revision:

  From code review:  use more robust OS check and move it to main()

-

Changes:
  - all: https://git.openjdk.org/jdk/pull/9061/files
  - new: https://git.openjdk.org/jdk/pull/9061/files/7cd2dccf..162e928c

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=9061=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=9061=00-01

  Stats: 6 lines in 1 file changed: 2 ins; 3 del; 1 mod
  Patch: https://git.openjdk.org/jdk/pull/9061.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9061/head:pull/9061

PR: https://git.openjdk.org/jdk/pull/9061


Re: RFR: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows

2022-06-09 Thread Naoto Sato
On Tue, 7 Jun 2022 12:19:29 GMT, Magnus Ihse Bursie  wrote:

> The test  
> `test/jdk/java/util/ResourceBundle/Control/MissingResourceCauseTest.java` 
> verifies different failure modes of resource bundles. One of the failures is 
> that the runner class, `MissingResourceCauseTestRun.java`, creates a file 
> `UnreadableRB`, and runs `chmod 000` on it, to make it unreadable by the 
> test. Then MissingResourceCauseTest is called, and the `UnreadableRB` file is 
> removed.
> 
> This does not work reliably on Windows. On msys2, `chmod` is essentially a 
> no-op, so the file is not made unreadable, and hence the test fails. In my 
> personal cygwin test environment, the chmod command does have some effect, 
> but it is still not enough to make the file unreadable, and so the test fails.
> 
> The test was originally a shell script test that got converted to Java in 
> [JDK-4354216](https://bugs.openjdk.org/browse/JDK-8213127). The original 
> shell script code explicitly excluded Windows from testing. This was changed 
> in the rewrite, for reasons I cannot determine.
> 
> What suprises me, though, is the "how can this ever has worked???" factor. 
> Apparently the test passes on the current Cygwin setup on GHA. I have failed 
> to reproduce the working conditions that makes a file actually unreadable for 
> the owner on Windows, neither on my GHA test repo, nor locally. I've searched 
> the web to figure out how to properly set file permissions on Windows to make 
> the file unreadable, using Windows native tools, to no avail. I've even asked 
> a [Stack Overflow 
> question](https://stackoverflow.com/questions/72528318/what-file-permissions-make-a-file-unreadable-by-owner-in-windows);
>  which as of yet is still unanswered.
> 
> Since I feel I've spent far more time than reasonable trying to get this to 
> work properly, I suggest we instead skip the unreadable test on Windows. It 
> is clearly unstable and highly depending on the Windows environment, the test 
> was never originally supported or intended for Windows, and at the of the 
> day, testing file unreadability is not an important regression test for 
> [JDK-4354216](https://bugs.openjdk.org/browse/JDK-4354216).

Marked as reviewed by naoto (Reviewer).

-

PR: https://git.openjdk.org/jdk/pull/9061


Re: RFR: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows

2022-06-09 Thread Naoto Sato
On Wed, 8 Jun 2022 17:43:49 GMT, Naoto Sato  wrote:

>> The test  
>> `test/jdk/java/util/ResourceBundle/Control/MissingResourceCauseTest.java` 
>> verifies different failure modes of resource bundles. One of the failures is 
>> that the runner class, `MissingResourceCauseTestRun.java`, creates a file 
>> `UnreadableRB`, and runs `chmod 000` on it, to make it unreadable by the 
>> test. Then MissingResourceCauseTest is called, and the `UnreadableRB` file 
>> is removed.
>> 
>> This does not work reliably on Windows. On msys2, `chmod` is essentially a 
>> no-op, so the file is not made unreadable, and hence the test fails. In my 
>> personal cygwin test environment, the chmod command does have some effect, 
>> but it is still not enough to make the file unreadable, and so the test 
>> fails.
>> 
>> The test was originally a shell script test that got converted to Java in 
>> [JDK-4354216](https://bugs.openjdk.org/browse/JDK-8213127). The original 
>> shell script code explicitly excluded Windows from testing. This was changed 
>> in the rewrite, for reasons I cannot determine.
>> 
>> What suprises me, though, is the "how can this ever has worked???" factor. 
>> Apparently the test passes on the current Cygwin setup on GHA. I have failed 
>> to reproduce the working conditions that makes a file actually unreadable 
>> for the owner on Windows, neither on my GHA test repo, nor locally. I've 
>> searched the web to figure out how to properly set file permissions on 
>> Windows to make the file unreadable, using Windows native tools, to no 
>> avail. I've even asked a [Stack Overflow 
>> question](https://stackoverflow.com/questions/72528318/what-file-permissions-make-a-file-unreadable-by-owner-in-windows);
>>  which as of yet is still unanswered.
>> 
>> Since I feel I've spent far more time than reasonable trying to get this to 
>> work properly, I suggest we instead skip the unreadable test on Windows. It 
>> is clearly unstable and highly depending on the Windows environment, the 
>> test was never originally supported or intended for Windows, and at the of 
>> the day, testing file unreadability is not an important regression test for 
>> [JDK-4354216](https://bugs.openjdk.org/browse/JDK-4354216).
>
> Thanks for the investigation, Magnus. I believe it is a bug to include the 
> Windows environment in refactoring the shell script to Java. Why it's been 
> working? No idea. Could be `IOException` has been thrown not by `unreadable` 
> but for some other reason?

> @naotoj Yes, that could be a likely possibility. Perhaps the file were never 
> created, or not in the expected place?

I'd guess something like that too. Anyways, eliminating this unreliability in 
test is a good clean up. Approving the fix assuming Jai's comment is addressed.

-

PR: https://git.openjdk.org/jdk/pull/9061


Re: RFR: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows

2022-06-09 Thread Magnus Ihse Bursie
On Wed, 8 Jun 2022 17:43:49 GMT, Naoto Sato  wrote:

>> The test  
>> `test/jdk/java/util/ResourceBundle/Control/MissingResourceCauseTest.java` 
>> verifies different failure modes of resource bundles. One of the failures is 
>> that the runner class, `MissingResourceCauseTestRun.java`, creates a file 
>> `UnreadableRB`, and runs `chmod 000` on it, to make it unreadable by the 
>> test. Then MissingResourceCauseTest is called, and the `UnreadableRB` file 
>> is removed.
>> 
>> This does not work reliably on Windows. On msys2, `chmod` is essentially a 
>> no-op, so the file is not made unreadable, and hence the test fails. In my 
>> personal cygwin test environment, the chmod command does have some effect, 
>> but it is still not enough to make the file unreadable, and so the test 
>> fails.
>> 
>> The test was originally a shell script test that got converted to Java in 
>> [JDK-4354216](https://bugs.openjdk.org/browse/JDK-8213127). The original 
>> shell script code explicitly excluded Windows from testing. This was changed 
>> in the rewrite, for reasons I cannot determine.
>> 
>> What suprises me, though, is the "how can this ever has worked???" factor. 
>> Apparently the test passes on the current Cygwin setup on GHA. I have failed 
>> to reproduce the working conditions that makes a file actually unreadable 
>> for the owner on Windows, neither on my GHA test repo, nor locally. I've 
>> searched the web to figure out how to properly set file permissions on 
>> Windows to make the file unreadable, using Windows native tools, to no 
>> avail. I've even asked a [Stack Overflow 
>> question](https://stackoverflow.com/questions/72528318/what-file-permissions-make-a-file-unreadable-by-owner-in-windows);
>>  which as of yet is still unanswered.
>> 
>> Since I feel I've spent far more time than reasonable trying to get this to 
>> work properly, I suggest we instead skip the unreadable test on Windows. It 
>> is clearly unstable and highly depending on the Windows environment, the 
>> test was never originally supported or intended for Windows, and at the of 
>> the day, testing file unreadability is not an important regression test for 
>> [JDK-4354216](https://bugs.openjdk.org/browse/JDK-4354216).
>
> Thanks for the investigation, Magnus. I believe it is a bug to include the 
> Windows environment in refactoring the shell script to Java. Why it's been 
> working? No idea. Could be `IOException` has been thrown not by `unreadable` 
> but for some other reason?

@naotoj Yes, that could be a likely possibility. Perhaps the file were never 
created, or not in the expected place?

-

PR: https://git.openjdk.java.net/jdk/pull/9061


Re: RFR: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows

2022-06-08 Thread Naoto Sato
On Tue, 7 Jun 2022 12:19:29 GMT, Magnus Ihse Bursie  wrote:

> The test  
> `test/jdk/java/util/ResourceBundle/Control/MissingResourceCauseTest.java` 
> verifies different failure modes of resource bundles. One of the failures is 
> that the runner class, `MissingResourceCauseTestRun.java`, creates a file 
> `UnreadableRB`, and runs `chmod 000` on it, to make it unreadable by the 
> test. Then MissingResourceCauseTest is called, and the `UnreadableRB` file is 
> removed.
> 
> This does not work reliably on Windows. On msys2, `chmod` is essentially a 
> no-op, so the file is not made unreadable, and hence the test fails. In my 
> personal cygwin test environment, the chmod command does have some effect, 
> but it is still not enough to make the file unreadable, and so the test fails.
> 
> The test was originally a shell script test that got converted to Java in 
> [JDK-4354216](https://bugs.openjdk.org/browse/JDK-8213127). The original 
> shell script code explicitly excluded Windows from testing. This was changed 
> in the rewrite, for reasons I cannot determine.
> 
> What suprises me, though, is the "how can this ever has worked???" factor. 
> Apparently the test passes on the current Cygwin setup on GHA. I have failed 
> to reproduce the working conditions that makes a file actually unreadable for 
> the owner on Windows, neither on my GHA test repo, nor locally. I've searched 
> the web to figure out how to properly set file permissions on Windows to make 
> the file unreadable, using Windows native tools, to no avail. I've even asked 
> a [Stack Overflow 
> question](https://stackoverflow.com/questions/72528318/what-file-permissions-make-a-file-unreadable-by-owner-in-windows);
>  which as of yet is still unanswered.
> 
> Since I feel I've spent far more time than reasonable trying to get this to 
> work properly, I suggest we instead skip the unreadable test on Windows. It 
> is clearly unstable and highly depending on the Windows environment, the test 
> was never originally supported or intended for Windows, and at the of the 
> day, testing file unreadability is not an important regression test for 
> [JDK-4354216](https://bugs.openjdk.org/browse/JDK-4354216).

Thanks for the investigation, Magnus. I believe it is a bug to include the 
Windows environment in refactoring the shell script to Java. Why it's been 
working? No idea. Could be `IOException` has been thrown not by `unreadable` 
but for some other reason?

-

PR: https://git.openjdk.java.net/jdk/pull/9061


Re: RFR: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows

2022-06-08 Thread Jaikiran Pai
On Tue, 7 Jun 2022 12:19:29 GMT, Magnus Ihse Bursie  wrote:

> I have failed to reproduce the working conditions that makes a file actually 
> unreadable for the owner on Windows, neither on my GHA test repo, nor 
> locally. 

I had quick look at the CI setup we have access to. It appears that this test 
passes there successfully on Windows. Could it be something to do with Windows 
version being used?

test/jdk/java/util/ResourceBundle/Control/MissingResourceCauseTest.java line 48:

> 46: private static void callGetBundle(String baseName, Locale locale,
> 47:   Class 
> expectedCause) {
> 48: if (baseName.equals("UnreadableRB") && 
> System.getProperty("os.name").startsWith("Windows")) {

Hello @magicus, perhaps we could do this check in the `main` of this class 
(line 37) before calling the `callGetBundle` for the `UnreadableRB` base name? 
That way this method doesn't have to bother about which `baseName` is being 
passed.

I had a quick look at how other tests do the OS name checks and they use the 
`Platform.isWindows()` API which is part of the test library. I suspect you may 
not be able to use it here though since this `MissingResourceCauseTest` class 
gets launched manually through the `MissingResourceCauseTestRun`. So using 
`System.getProperty` I think is fine. Nit - perhaps we could match this check 
to do whatever `Platform.isWindows()` is doing (which seems to be checking that 
the os name starts with `win`, ignoring the case).

-

PR: https://git.openjdk.java.net/jdk/pull/9061


RFR: 8287902: UnreadableRB case in MissingResourceCauseTest is not working reliably on Windows

2022-06-07 Thread Magnus Ihse Bursie
The test  
`test/jdk/java/util/ResourceBundle/Control/MissingResourceCauseTest.java` 
verifies different failure modes of resource bundles. One of the failures is 
that the runner class, `MissingResourceCauseTestRun.java`, creates a file 
`UnreadableRB`, and runs `chmod 000` on it, to make it unreadable by the test. 
Then MissingResourceCauseTest is called, and the `UnreadableRB` file is removed.

This does not work reliably on Windows. On msys2, `chmod` is essentially a 
no-op, so the file is not made unreadable, and hence the test fails. In my 
personal cygwin test environment, the chmod command does have some effect, but 
it is still not enough to make the file unreadable, and so the test fails.

The test was originally a shell script test that got converted to Java in 
[JDK-4354216](https://bugs.openjdk.org/browse/JDK-8213127). The original shell 
script code explicitly excluded Windows from testing. This was changed in the 
rewrite, for reasons I cannot determine.

What suprises me, though, is the "how can this ever has worked???" factor. 
Apparently the test passes on the current Cygwin setup on GHA. I have failed to 
reproduce the working conditions that makes a file actually unreadable for the 
owner on Windows, neither on my GHA test repo, nor locally. I've searched the 
web to figure out how to properly set file permissions on Windows to make the 
file unreadable, using Windows native tools, to no avail. I've even asked a 
[Stack Overflow 
question](https://stackoverflow.com/questions/72528318/what-file-permissions-make-a-file-unreadable-by-owner-in-windows);
 which as of yet is still unanswered.

Since I feel I've spent far more time than reasonable trying to get this to 
work properly, I suggest we instead skip the unreadable test on Windows. It is 
clearly unstable and highly depending on the Windows environment, the test was 
never originally supported or intended for Windows, and at the of the day, 
testing file unreadability is not an important regression test for 
[JDK-4354216](https://bugs.openjdk.org/browse/JDK-4354216).

-

Commit messages:
 - 8287902: UnreadableRB case in MissingResourceCauseTest is not working 
reliably on Windows

Changes: https://git.openjdk.java.net/jdk/pull/9061/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=9061=00
  Issue: https://bugs.openjdk.org/browse/JDK-8287902
  Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/9061.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/9061/head:pull/9061

PR: https://git.openjdk.java.net/jdk/pull/9061