8366733: Re-examine older java.text NF, DF, and DFS serialization tests

[justin-curtis-lu]

This PR cleans up the existing DecimalFormatSymbols, DecimalFormat, and NumberFormat serialization tests.

As mentioned in #27008 these tests can be re-visited.

These older tests are either not run (since they rely on being run by older JDK versions), rely on hex dump files, or are simply outdated. These are now removed or updated and moved under DecimalFormat/SerializationTest.java and DFSSerializationTest.java. Additionally, tests to check the stream version invariants are added for DecimalFormat.

Below are the existing tests that are removed or modified. (Their associated hex dump files are removed as well).

DFSDeserialization142.java & DFSSerialization142.java (D)

• They do not have Jtreg headers and are not run. The comments indicate they require a specific JDK version of 1.4.2. Instead, a currency symbol test is added in place.

NumberFormat/DFSSerialization.java (D)

• Test 1 checks if a DFS written from a 1.4.2 JDK with stream version of 2 when read has the correct String exponent and currency symbol. -> There is an existing test which checks the exponent. A New test is added in place of currency symbol test.
• Test 2 checks that a DFS maintains the exponent separator and currency symbol when read. -> Tests are added in place of them of them.
• Test 3 is unrelated to de serialization, and checks that the exponent separator symbol setter throws NPE. -> Test 3 is already covered by SettersShouldThrowNPETest.java.

SerializationLoad.java & SerializationSave.java (D)

• The save test depends on JDK 1.1.4, and is the code to write a DFS and DF. (It has no header and is not run.) The load test uses the saved 1.1.4 hex dump file to ensure the DFS and DF can be read, it did not check any specific invariants.

NumberRegression.java (M)

• Test4185761 and Test4069754 check NF/DF invariants and are ported to the dedicated serialization test file.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8366733: Re-examine older java.text NF, DF, and DFS serialization tests (Bug - P4)

Reviewers

• Naoto Sato (@naotoj - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27108/head:pull/27108
$ git checkout pull/27108

Update a local copy of the PR:
$ git checkout pull/27108
$ git pull https://git.openjdk.org/jdk.git pull/27108/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27108

View PR using the GUI difftool:
$ git pr show -t 27108

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27108.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back jlu! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@justin-curtis-lu This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8366733: Re-examine older java.text NF, DF, and DFS serialization tests

Reviewed-by: naoto

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 47 new commits pushed to the master branch:

• cdc8b5e: 8366455: Move VarHandles.GuardMethodGenerator to execute on build
• 1ebe949: 8314488: Compiling the JDK with C++17
• b674a42: 8366750: Remove test 'java/awt/Choice/ChoiceMouseWheelTest/ChoiceMouseWheelTest.java' from problemlist
• ... and 44 more: https://git.openjdk.org/jdk/compare/e3b36e3babb860d9d24a610160f47d42cfaafaa3...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@justin-curtis-lu The following labels will be automatically applied to this pull request:

• core-libs
• i18n

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 02: Full - Incremental (c57082c6)
• 01: Full - Incremental (59f07e9d)
• 00: Full (95c3c0c7)

[RogerRiggs]

Have you verified that the serialized streams in these new tests are identical to the hexformat versions?

[naotoj]

Although the existing tests are old and not run, I think the existing data can be reused. You can create overload for deSer() for these and test them. I believe they are the real serialized objects from the older releases, so I believe testing them still have value.

[justin-curtis-lu]

Have you verified that the serialized streams in these new tests are identical to the hexformat versions?

I looked at the respective Serialization save files and what was being tested in the load files to determine the configuration of the replacement tests. I stepped through the tests with a debugger to make sure the data read from the stream that was being tested was accurate with what was indicated in the save file. I do not expect the streams themselves to be identical (since the new ones are mocked), but rather the invariants being tested should be the same.

[justin-curtis-lu]

Although the existing tests are old and not run, I think the existing data can be reused. You can create overload for deSer() for these and test them. I believe they are the real serialized objects from the older releases, so I believe testing them still have value.

The new tests mock these streams so that we could do away with relying on the hex dump files (which required a user to navigate to the unused save file to determine how the stream was created). What is being tested should be the same for the mocked stream and the original stream itself. However, if it is desirable to test the exact streams from older releases, I can bring the data files back and test their streams directly.

[naotoj]

However, if it is desirable to test the exact streams from older releases, I can bring the data files back and test their streams directly.

Yes, I think so. Unless there is any specific reason to remove tests, it is the main purpose for these regression tests.

[RogerRiggs]

Yes, the purpose of the hex dumps is to test exactly the stream produced by earlier versions.
They should be retained.

[justin-curtis-lu]

Brought the hex dump files back (more appropriately under the DecimalFormat folder) and retained the original tests.

[naotoj]

LGTM. Thanks for revisiting the serialization tests.

[naotoj]

Nit: since the content of the file has not changed, I don't think we need to update the year

[justin-curtis-lu]

Restored in c57082c

[justin-curtis-lu]

/integrate

[openjdk]

Going to push as commit 24a7349.
Since your change was applied there have been 89 commits pushed to the master branch:

• a12e9fc: 8366261: Provide utility methods for sun.security.util.Password
• cc6d34b: 8366971: C2: Remove unused nop_list from PhaseOutput::init_buffer
• b653ae9: 8367051: Build failure with clang on linux and AIX after switch to C++17
• ... and 86 more: https://git.openjdk.org/jdk/compare/e3b36e3babb860d9d24a610160f47d42cfaafaa3...master

Your commit was automatically rebased without conflicts.

[openjdk]

@justin-curtis-lu Pushed as commit 24a7349.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.