RFR: 8189354: ArrayIndexOutOfBoundsException when listening to selection changes on TreeTableView [v3]

mstr2 github.com+43553916+mstr2 at openjdk.java.net
Tue Apr 27 16:33:38 UTC 2021


On Tue, 27 Apr 2021 10:18:24 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

> A couple of notes to your test - my personal preferences:
> 
> * they should cover all changes, that is test both Tree- and TreeTableView
> * if possible, have a test that really cover the report: here that would be throwing a AIOOP on expanding again (with a filter to re-select all children)
> * have a test for underlying reason of the reported misbehavior: here the incorrect change
> * testing the change should be complete (in testing its state)
> 
> I have seen you using the MockListObserver in an earlier version of this PR - which is perfect for the last bullet, we should use it always in asserting the correctness of listChange notification :) The only very minor drawback is that it requires to update the Eclipse classpath to add-exports for the base testing package (not a big deal).

I think you're right that the same test could also be added for TreeTableView, and that the test should probably cover the entirety of the change notification. However, my preference is to not create a test that duplicates all aspects of the specific program that was attached to the JBS issue. I think it's more meaningful to test the most minimal setup that can reproduce the underlying issue; adding bells and whistles (as was done in the JBS code sample) feels like adding unnecessary noise to the test.

-------------

PR: https://git.openjdk.java.net/jfx/pull/480


More information about the openjfx-dev mailing list