On Wed, 14 Feb 2024 23:05:25 GMT, Justin Lu <j...@openjdk.org> wrote:
> Please review this PR which fixes / finishes the rest of the IntlTest test
> framework removal in java.text and java.util.i18n tests.
>
> For context, the IntlTest class only ran methods prefixed by _test_ or _Test_
> with public visibility and was originally removed due to some tests
> spuriously passing that were not aware of the specific running requirements.
>
> This PR includes the following changes,
> - There were some tests with package-private visibility that were never ran
> by IntlTest. Those tests do appear to be valid tests, and thus have been
> updated with `@Test` annotation.
> - The test method DFSExponenTest() in DFSExponential.java was not prefixed by
> test and thus never ran by IntlTest. It is a valid test and should be ran as
> well.
> - DateFormatRoundTripTest.java was supposed to remain a non JUnit test,
> however the run invocation was converted to `@run junit ...` this has been
> switched back to `@run main ...`
> - There were two instances of the script missing some tests that should have
> had their methods updated with the `@Test` annotation: APITest.java and
> DFSSerialization.java. These tests have been updated accordingly.
LGTM
test/jdk/java/text/Format/NumberFormat/DFSExponential.java line 44:
> 42:
> 43: @Test
> 44: public void DFSExponenTest() throws Exception {
Not your change, but the method name reads odd to me. `TestDFSExponential` may
be better alined with the other test for serialization.
-------------
Marked as reviewed by naoto (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/17858#pullrequestreview-1881533695
PR Review Comment: https://git.openjdk.org/jdk/pull/17858#discussion_r1490216492
