On Fri, 19 Jun 2020 23:49:45 GMT, Kevin Rushforth <[email protected]> wrote:
>> Node.getPseudoClassStates() returns a new UnmodifiableObservableSet of >> PseudoClassState on each call. So in order to >> listen to any changes in this set, user must call the method >> Node.getPseudoClassStates() only once and keep a strong >> reference to the returned UnmodifiableObservableSet. >> >> So the fix is that the method Node.getPseudoClassStates() should return the >> same UnmodifiableObservableSet on every >> call. As the returned set is an UnmodifiableObservableSet, it will not have >> any impact on it's usage. >> >> Added a small unit test. and, >> Altered(minor) a test which was modified in >> past(https://github.com/openjdk/jfx/commit/0ac98695a1b11443c342fad4f009d6e03a052522) >> (https://github.com/openjdk/jfx/commit/62323e0a9c5817b33daa262d6914eba0e8d274ff) >> along with this method and should be >> updated in view of this change. > > 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)); } ------------- PR: https://git.openjdk.java.net/jfx/pull/253
