RFR: 8261919: java/util/Locale/LocaleProvidersRun.java failed with "RuntimeException: Expected log was not emitted. LogRecord: null"
Please review the fix to this test case failure that occurs with the usage tracker enabled JRE. ------------- Commit messages: - 8261919: java/util/Locale/LocaleProvidersRun.java failed with "RuntimeException: Expected log was not emitted. LogRecord: null" Changes: https://git.openjdk.java.net/jdk/pull/2683/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2683&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8261919 Stats: 31 lines in 2 files changed: 17 ins; 7 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/2683.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2683/head:pull/2683 PR: https://git.openjdk.java.net/jdk/pull/2683
On Tue, 23 Feb 2021 02:09:01 GMT, Naoto Sato <naoto@openjdk.org> wrote:
Please review the fix to this test case failure that occurs with the usage tracker enabled JRE.
Marked as reviewed by joehw (Reviewer). ------------- PR: https://git.openjdk.java.net/jdk/pull/2683
On Tue, 23 Feb 2021 02:09:01 GMT, Naoto Sato <naoto@openjdk.org> wrote:
Please review the fix to this test case failure that occurs with the usage tracker enabled JRE.
test/jdk/java/util/Locale/LocaleProviders.java line 416:
414: // Set the root logger on loading the logging class 415: public static class LogConfig { 416: final static LogRecord[] lra = new LogRecord[1];
I would suggest to use some multi-thread safe container rather than a simple array to store the LogRecord. For instance - a CopyOnWriteArrayList - or something like that. Also you may want to harden the test by allowing for the possibility that some other logging might have occurred, and search the list for the record you expect rather than assuming it will be the first and only one. Otherwise looks good! ------------- PR: https://git.openjdk.java.net/jdk/pull/2683
On Tue, 23 Feb 2021 09:49:47 GMT, Daniel Fuchs <dfuchs@openjdk.org> wrote:
Please review the fix to this test case failure that occurs with the usage tracker enabled JRE.
test/jdk/java/util/Locale/LocaleProviders.java line 416:
414: // Set the root logger on loading the logging class 415: public static class LogConfig { 416: final static LogRecord[] lra = new LogRecord[1];
I would suggest to use some multi-thread safe container rather than a simple array to store the LogRecord. For instance - a CopyOnWriteArrayList - or something like that. Also you may want to harden the test by allowing for the possibility that some other logging might have occurred, and search the list for the record you expect rather than assuming it will be the first and only one. Otherwise looks good!
Thanks, Daniel. Will update the fix as you suggested. ------------- PR: https://git.openjdk.java.net/jdk/pull/2683
Please review the fix to this test case failure that occurs with the usage tracker enabled JRE.
Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Reflecting review comments. ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/2683/files - new: https://git.openjdk.java.net/jdk/pull/2683/files/bfa3a8e5..4d82e8aa Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2683&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2683&range=00-01 Stats: 9 lines in 1 file changed: 1 ins; 2 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/2683.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2683/head:pull/2683 PR: https://git.openjdk.java.net/jdk/pull/2683
On Tue, 23 Feb 2021 19:35:55 GMT, Naoto Sato <naoto@openjdk.org> wrote:
Please review the fix to this test case failure that occurs with the usage tracker enabled JRE.
Naoto Sato has updated the pull request incrementally with one additional commit since the last revision:
Reflecting review comments.
LGTM! ------------- Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2683
On Tue, 23 Feb 2021 02:09:01 GMT, Naoto Sato <naoto@openjdk.org> wrote:
Please review the fix to this test case failure that occurs with the usage tracker enabled JRE.
This pull request has now been integrated. Changeset: 9d9ad969 Author: Naoto Sato <naoto@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/9d9ad969 Stats: 31 lines in 2 files changed: 16 ins; 7 del; 8 mod 8261919: java/util/Locale/LocaleProvidersRun.java failed with "RuntimeException: Expected log was not emitted. LogRecord: null" Reviewed-by: joehw, dfuchs ------------- PR: https://git.openjdk.java.net/jdk/pull/2683
participants (3)
-
Daniel Fuchs
-
Joe Wang
-
Naoto Sato