Re: RFR: 8283349 : Robustness improvements to java/util/prefs/AddNodeChangeListener.jar [v2]

2022-03-25 Thread Lance Andersen
On Thu, 24 Mar 2022 20:59:20 GMT, Brent Christian  wrote:

>> Please review this change to the java/util/prefs/AddNodeChangeListener.jar 
>> test.
>> 
>> Although the test specifies "-Djava.util.prefs.userRoot=." to ensure a fresh 
>> Preferences on each test run, MacOS does not seem to honor this, and still 
>> stores prefs in Library/.
>> 
>> This test has long suffered intermittent failures. 8255031 added some 
>> debugging code; as of that change, the test fails fast as soon as an 
>> expected change event is not received. In particular, if the expected event 
>> is not received for adding the node, the test exits without removing the 
>> node. In this way, on Mac, the node can get "stuck" in the preferences of 
>> the machine. Future runs on the machine will already have the node, no node 
>> added change event will be generated (because the node was already there), 
>> the test will continue to fail without removing the node, etc. This matches 
>> observations on some CI machines.  
>> 
>> This change ensures that:
>> * the test node is not present before the test
>> * the test runs to completion, including removing the test node
>> * the test node is not left behind after the test
>> 
>> Thanks.
>
> Brent Christian has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Add test failure message to 'couldn't remove' RuntimeException

Marked as reviewed by lancea (Reviewer).

-

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


Re: RFR: 8283349 : Robustness improvements to java/util/prefs/AddNodeChangeListener.jar [v2]

2022-03-24 Thread Brent Christian
On Wed, 23 Mar 2022 17:29:04 GMT, Naoto Sato  wrote:

>> Brent Christian has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Add test failure message to 'couldn't remove' RuntimeException
>
> test/jdk/java/util/prefs/AddNodeChangeListener.java line 63:
> 
>> 61: } finally {
>> 62: // Make sure test node is not present after the test
>> 63: clearPrefs();
> 
> If this call throws a `RuntimeException("Unable to clear...")`, it will 
> override the possible `RuntimeException("Failed")` in the `try` block, but I 
> think it is OK.

That's true - I hadn't considered that. I think it's OK, too. Even so, I've 
added a message to the exception thrown from clearPrefs() to indicate if the 
test failed.

-

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


Re: RFR: 8283349 : Robustness improvements to java/util/prefs/AddNodeChangeListener.jar [v2]

2022-03-24 Thread Brent Christian
> Please review this change to the java/util/prefs/AddNodeChangeListener.jar 
> test.
> 
> Although the test specifies "-Djava.util.prefs.userRoot=." to ensure a fresh 
> Preferences on each test run, MacOS does not seem to honor this, and still 
> stores prefs in Library/.
> 
> This test has long suffered intermittent failures. 8255031 added some 
> debugging code; as of that change, the test fails fast as soon as an expected 
> change event is not received. In particular, if the expected event is not 
> received for adding the node, the test exits without removing the node. In 
> this way, on Mac, the node can get "stuck" in the preferences of the machine. 
> Future runs on the machine will already have the node, no node added change 
> event will be generated (because the node was already there), the test will 
> continue to fail without removing the node, etc. This matches observations on 
> some CI machines.  
> 
> This change ensures that:
> * the test node is not present before the test
> * the test runs to completion, including removing the test node
> * the test node is not left behind after the test
> 
> Thanks.

Brent Christian has updated the pull request incrementally with one additional 
commit since the last revision:

  Add test failure message to 'couldn't remove' RuntimeException

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7908/files
  - new: https://git.openjdk.java.net/jdk/pull/7908/files/71ab1f17..139b6a94

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

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7908.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7908/head:pull/7908

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