RFR: 8214699: Node.getPseudoClassStates must return the same instance on every call [v2]
Ambarish Rapte
arapte at openjdk.java.net
Tue Jun 23 10:33:27 UTC 2020
On Mon, 22 Jun 2020 12:08:21 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:
>> modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java line 168:
>>
>>> 167: assertSame("getPseudoClassStates() should always return the same instance",
>>> 168: node.getPseudoClassStates(), node.getPseudoClassStates());
>>> 169: }
>>
>> To make this more clear, I recommend to store the expected value in a local variable and then call assert.
>
> would suggest some more tests (overcautious maybe, be have seen horses vomit at unbelievable places ;)
>
> Test that the returned set
>
> - is indeed unmodifiable
> - not gc'ed
> - is a wrapper around the backing set
>
> Test code:
>
> @Test(expected=UnsupportedOperationException.class)
> public void testPseudoClassesUnmodifiable() {
> Node node = new Rectangle();
> node.getPseudoClassStates().add(PseudoClass.getPseudoClass("dummy"));
> }
>
> @Test
> public void testPseudoClassesNotGCed() {
> Node node = new Rectangle();
> WeakReference<Set<?>> weakRef = new WeakReference<>(node.getPseudoClassStates());
> ControlSkinFactory.attemptGC(weakRef);
> assertNotNull("pseudoClassStates must not be gc'ed", weakRef.get());
> }
>
> // requires additional method in NodeShim
> @Test
> public void testUnmodifiablePseudoClassStatesEqualsBackingStates() {
> Rectangle node = new Rectangle();
> PseudoClass pseudo = PseudoClass.getPseudoClass("somepseudo");
> node.pseudoClassStateChanged(pseudo, true);
> assertEquals(1, node.getPseudoClassStates().size());
> assertEquals(NodeShim.pseudoClassStates(node).size(), node.getPseudoClassStates().size());
> assertTrue(NodeShim.pseudoClassStates(node).contains(pseudo));
> assertTrue(node.getPseudoClassStates().contains(pseudo));
> }
Corrected the existing test and added these new tests, with minor changes.
ControlSkinFactory is a file from control tests. So have added a copy of `attemptGC()` method in
`test.javafx.scene.shape.TestUtils` class. The class does not seem to be a perfect location for `attemptGC()`. But
wanted to avoid a new util kind off class just for this one method.
-------------
PR: https://git.openjdk.java.net/jfx/pull/253
More information about the openjfx-dev
mailing list