On Wed, 17 Jun 2020 11:46:07 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
>> modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewTest.java >> line 472: >> >>> 471: countSelectedIndexChangeEvent++; >>> 472: assertEquals(selectedItemBefore, >>> treeTableView.getTreeItem(sm.getSelectedIndex())); >>> 473: }); >> >> If this assertion ever fails, I don't think it will cause a test failure, >> since the event handling code will swallow >> the exception. The same is true of the other listeners. I don't know if it >> would be possible to use the >> UncaughtExceptionHandler as is done in other tests -- see >> [JDK-8244531](https://bugs.openjdk.java.net/browse/JDK-8244531) -- but that >> might be worth exploring. Another >> possibility is to wrap all the listeners in a try/catch and keep a list of >> `Throwable`s that are caught. > > Hi Kevin, I tested these listeners by adding assert that always fail. It > shows the failures correctly. Also in this > file there are other tests which assert inside a listener. I confirmed that > those tests also show failures correctly. I > could not find out what makes it work though. I can confirm that the test fails if an assertion hits. The reason is that this ChangeListener throws any exception to the code that causes the change, so it will propagate to the test thread. >> modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewTest.java >> line 431: >> >>> 430: for (int j = 0; j < FIRST_LEVEL_COUNT - 1; j++) { >>> 431: TreeItem<String> tj = new TreeItem<>("" + i + j); >>> 432: tj.setExpanded(true); >> >> The tree item strings will not be unique. This won't affect whether the test >> passes or fails, since `TreeItem` does not >> override `equals`, but it might be better if the strings were unique (in >> case there ever was an error it would be >> easier to understand). > > Hi Kevin, I verified the the tree item strings again. They seem to be unique. > A total of 8800 of tree items get created > each with unique string. Could you please recheck. Confirmed. This was my mistake. ------------- PR: https://git.openjdk.java.net/jfx/pull/244
