Re: RFR: 8283349 : Robustness improvements to java/util/prefs/AddNodeChangeListener.jar [v2]
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]
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]
> 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