Re: RFR: 8313612: Use JUnit in lib-test/jdk tests [v4]
On Tue, 19 Sep 2023 18:30:10 GMT, Qing Xiao wrote: >> Modified all tests under lib-test/jdk to use JUnit > > Qing Xiao has updated the pull request incrementally with three additional > commits since the last revision: > > - Update test/lib-test/jdk/test/lib/hexdump/ObjectStreamPrinterTest.java > >Co-authored-by: liach <7806504+li...@users.noreply.github.com> > - Update test/lib-test/jdk/test/lib/hexdump/HexPrinterTest.java > >Co-authored-by: liach <7806504+li...@users.noreply.github.com> > - Update test/lib-test/jdk/test/lib/format/ArrayDiffTest.java > >Co-authored-by: Christian Stein Marked as reviewed by asotona (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/15131#pullrequestreview-1636039145
Re: RFR: 8313612: Use JUnit in lib-test/jdk tests [v4]
On Tue, 19 Sep 2023 18:30:10 GMT, Qing Xiao wrote: >> Modified all tests under lib-test/jdk to use JUnit > > Qing Xiao has updated the pull request incrementally with three additional > commits since the last revision: > > - Update test/lib-test/jdk/test/lib/hexdump/ObjectStreamPrinterTest.java > >Co-authored-by: liach <7806504+li...@users.noreply.github.com> > - Update test/lib-test/jdk/test/lib/hexdump/HexPrinterTest.java > >Co-authored-by: liach <7806504+li...@users.noreply.github.com> > - Update test/lib-test/jdk/test/lib/format/ArrayDiffTest.java > >Co-authored-by: Christian Stein Looks good to me. Tested with `--test test/lib-test/jdk/test/lib`. - Marked as reviewed by cstein (Committer). PR Review: https://git.openjdk.org/jdk/pull/15131#pullrequestreview-1635286817
Re: RFR: 8313612: Use JUnit in lib-test/jdk tests [v4]
> Modified all tests under lib-test/jdk to use JUnit Qing Xiao has updated the pull request incrementally with three additional commits since the last revision: - Update test/lib-test/jdk/test/lib/hexdump/ObjectStreamPrinterTest.java Co-authored-by: liach <7806504+li...@users.noreply.github.com> - Update test/lib-test/jdk/test/lib/hexdump/HexPrinterTest.java Co-authored-by: liach <7806504+li...@users.noreply.github.com> - Update test/lib-test/jdk/test/lib/format/ArrayDiffTest.java Co-authored-by: Christian Stein - Changes: - all: https://git.openjdk.org/jdk/pull/15131/files - new: https://git.openjdk.org/jdk/pull/15131/files/e520735d..7043c900 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15131=03 - incr: https://webrevs.openjdk.org/?repo=jdk=15131=02-03 Stats: 3 lines in 3 files changed: 0 ins; 2 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/15131.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15131/head:pull/15131 PR: https://git.openjdk.org/jdk/pull/15131
Re: RFR: 8313612: Use JUnit in lib-test/jdk tests [v3]
On Thu, 7 Sep 2023 07:15:06 GMT, Qing Xiao wrote: >> Modified all tests under lib-test/jdk to use JUnit > > Qing Xiao has updated the pull request incrementally with one additional > commit since the last revision: > > Change test static method to instance method test/lib-test/jdk/test/lib/format/ArrayDiffTest.java line 2: > 1: /* > 2: * Copyright (c) 2020, 2021, 2023, Oracle and/or its affiliates. All > rights reserved. Suggestion: * Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved. - PR Review Comment: https://git.openjdk.org/jdk/pull/15131#discussion_r1329896061
Re: RFR: 8313612: Use JUnit in lib-test/jdk tests [v3]
On Thu, 7 Sep 2023 07:15:06 GMT, Qing Xiao wrote: >> Modified all tests under lib-test/jdk to use JUnit > > Qing Xiao has updated the pull request incrementally with one additional > commit since the last revision: > > Change test static method to instance method test/lib-test/jdk/test/lib/hexdump/HexPrinterTest.java line 214: > 212: } > 213: > 214: // PrimitiveFormatters This comment is now useless. Suggestion: test/lib-test/jdk/test/lib/hexdump/ObjectStreamPrinterTest.java line 90: > 88: } > 89: > 90: // SingleObjects Suggestion: - PR Review Comment: https://git.openjdk.org/jdk/pull/15131#discussion_r1329887061 PR Review Comment: https://git.openjdk.org/jdk/pull/15131#discussion_r1329888251
Re: RFR: 8313612: Use JUnit in lib-test/jdk tests [v3]
On Tue, 15 Aug 2023 08:07:09 GMT, Christian Stein wrote: >> test/lib-test/jdk/test/lib/format/ArrayDiffTest.java line 2: >> >>> 1: /* >>> 2: * Copyright (c) 2020, 2021, 2023, Oracle and/or its affiliates. All >>> rights reserved. >> >> Should it just modify the second year, here is `2021`, to `2023`? > > Yes, good catch! I added two more occurances to my second review. The line must only two years. Like so: ` * Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved.` - PR Review Comment: https://git.openjdk.org/jdk/pull/15131#discussion_r1329878287
Re: RFR: 8313612: Use JUnit in lib-test/jdk tests [v3]
> Modified all tests under lib-test/jdk to use JUnit Qing Xiao has updated the pull request incrementally with one additional commit since the last revision: Change test static method to instance method - Changes: - all: https://git.openjdk.org/jdk/pull/15131/files - new: https://git.openjdk.org/jdk/pull/15131/files/0e15e752..e520735d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15131=02 - incr: https://webrevs.openjdk.org/?repo=jdk=15131=01-02 Stats: 13 lines in 4 files changed: 0 ins; 0 del; 13 mod Patch: https://git.openjdk.org/jdk/pull/15131.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15131/head:pull/15131 PR: https://git.openjdk.org/jdk/pull/15131
Re: RFR: 8313612: Use JUnit in lib-test/jdk tests
On Wed, 2 Aug 2023 23:25:13 GMT, Qing Xiao wrote: > Modified all tests under lib-test/jdk to use JUnit Should all static `@Test` methods be converted to instance methods, as recommended by JUnit? See https://junit.org/junit5/docs/snapshot/user-guide/#writing-tests-definitions > Test Method any **instance** method that is directly annotated or meta-annotated with `@Test`, ... - PR Comment: https://git.openjdk.org/jdk/pull/15131#issuecomment-1679037063
Re: RFR: 8313612: Use JUnit in lib-test/jdk tests [v2]
> Modified all tests under lib-test/jdk to use JUnit Qing Xiao has updated the pull request incrementally with three additional commits since the last revision: - Delete extra space Co-authored-by: Andrey Turbanov - Update years in comments in test/lib-test/jdk/test/lib/hexdump Co-authored-by: Christian Stein - Update years in comments in test/lib-test/jdk/test/lib/hexdump Co-authored-by: Christian Stein - Changes: - all: https://git.openjdk.org/jdk/pull/15131/files - new: https://git.openjdk.org/jdk/pull/15131/files/1ab34e8d..0e15e752 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=15131=01 - incr: https://webrevs.openjdk.org/?repo=jdk=15131=00-01 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/15131.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15131/head:pull/15131 PR: https://git.openjdk.org/jdk/pull/15131
Re: RFR: 8313612: Use JUnit in lib-test/jdk tests
On Wed, 2 Aug 2023 23:25:13 GMT, Qing Xiao wrote:
> Modified all tests under lib-test/jdk to use JUnit
test/lib-test/jdk/test/lib/hexdump/HexPrinterTest.java line 84:
> 82: Arguments.of("canonical", "%08x ", "%02x ", 16, "|", 31,
> HexPrinter.Formatters.PRINTABLE, "|" + System.lineSeparator()),
> 83: Arguments.of("simple", "%04x: ", "%02x ", 16, " // ", 64,
> HexPrinter.Formatters.ASCII, System.lineSeparator()),
> 84: Arguments.of("source", "", "(byte)%3d, ", 8, " // ",
> 64, HexPrinter.Formatters.PRINTABLE, System.lineSeparator())
Suggestion:
Arguments.of("source", "", "(byte)%3d, ", 8, " // ", 64,
HexPrinter.Formatters.PRINTABLE, System.lineSeparator())
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15131#discussion_r1294433943
Re: RFR: 8313612: Use JUnit in lib-test/jdk tests
On Wed, 2 Aug 2023 23:25:13 GMT, Qing Xiao wrote: > Modified all tests under lib-test/jdk to use JUnit Please only keep first and last/latest year of changes. test/lib-test/jdk/test/lib/hexdump/HexPrinterTest.java line 2: > 1: /* > 2: * Copyright (c) 2019, 2020, 2023, Oracle and/or its affiliates. All > rights reserved. Suggestion: * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved. test/lib-test/jdk/test/lib/hexdump/ObjectStreamPrinterTest.java line 2: > 1: /* > 2: * Copyright (c) 2019, 2021, 2023, Oracle and/or its affiliates. All > rights reserved. Suggestion: * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved. - Changes requested by cstein (Committer). PR Review: https://git.openjdk.org/jdk/pull/15131#pullrequestreview-1578134852 PR Review Comment: https://git.openjdk.org/jdk/pull/15131#discussion_r1294308211 PR Review Comment: https://git.openjdk.org/jdk/pull/15131#discussion_r1294308602
Re: RFR: 8313612: Use JUnit in lib-test/jdk tests
On Tue, 15 Aug 2023 08:03:59 GMT, John Jiang wrote: >> Modified all tests under lib-test/jdk to use JUnit > > test/lib-test/jdk/test/lib/format/ArrayDiffTest.java line 2: > >> 1: /* >> 2: * Copyright (c) 2020, 2021, 2023, Oracle and/or its affiliates. All >> rights reserved. > > Should it just modify the second year, here is `2021`, to `2023`? Yes, good catch! - PR Review Comment: https://git.openjdk.org/jdk/pull/15131#discussion_r1294306627
Re: RFR: 8313612: Use JUnit in lib-test/jdk tests
On Wed, 2 Aug 2023 23:25:13 GMT, Qing Xiao wrote: > Modified all tests under lib-test/jdk to use JUnit test/lib-test/jdk/test/lib/format/ArrayDiffTest.java line 2: > 1: /* > 2: * Copyright (c) 2020, 2021, 2023, Oracle and/or its affiliates. All > rights reserved. Should it just modify the second year, here is `2021`, to `2023`? - PR Review Comment: https://git.openjdk.org/jdk/pull/15131#discussion_r1294304121
Re: RFR: 8313612: Use JUnit in lib-test/jdk tests
On Wed, 2 Aug 2023 23:25:13 GMT, Qing Xiao wrote: > Modified all tests under lib-test/jdk to use JUnit Marked as reviewed by cstein (Committer). - PR Review: https://git.openjdk.org/jdk/pull/15131#pullrequestreview-1578082673
