On Fri, 10 Dec 2021 17:03:49 GMT, Liam Miller-Cushon <cus...@openjdk.org> wrote:
>> This change updates the serialized objects used by >> `java/awt/dnd/BadSerializationTest/BadSerializationTest.java` using a >> similar approach to the previous fix in >> [JDK-8039082](https://bugs.openjdk.java.net/browse/JDK-8039082). > > Liam Miller-Cushon has updated the pull request incrementally with one > additional commit since the last revision: > > Add a few more missing calls to LookAndFeel.uninstallColors I've been contemplating about what is being done in this PR… It feels wrong. > > As of the usefulness of the serializations tests in Swing - it is a good > > coverage of the cleaning code in the L&fs. Each L&F should clean all its > > data when the app switch one LF to another, but if old L&F forget to > > cleanup then the app will get a memory leak. Swing explicitly warns, “Serialized objects of this class will not be compatible with future Swing releases.” Don't break this rule in this test? A component is serialized using one version of the JDK and is deserialized in another version of the JDK and even on another platform. > This test doesn't switch L&Fs .. if it wasn't for mac defaulting to Aqua > they'd all be using Metal. > > So _this is an accidental by-product_, not the goal of the test. It looks so. If the L&Fs are expected to clean up when uninstalled, it should be tested thoroughly for all the components and Look-and-Feels. Only this way we can be sure the properties get reset. While working on this code review, Liam has found and fixed quite a few places where clean-up wasn't done properly. > And that goal be met by the test writing out the serialized objects and > reading them back all at "test time", can't it ? > > Seems to me right now we are stumbling over cases where what is serialized is > inappropriate for persistent serialization. Taking into account the above comments, this test could be simplified to write out the objects and then read them in. No persistent serialization which is _explicitly_ unsupported. To be clear, I think the proposed change is good on its own: it adds convenience methods to clean up UI properties when a L&F is uninstalled, it adds the clean-up where it was missing. But it doesn't fit into the fix of this test failure. This work should be an enhancement performed under another bugid. ------------- PR: https://git.openjdk.java.net/jdk/pull/6603
