On Wed, 24 Aug 2022 22:03:26 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> The issue is caused by TreeTableRow incorrectly selected when cell selection >> mode is enabled. >> >> Changes: >> - modified TreeTableRow.updateSelection() > > Andy Goryachev has updated the pull request incrementally with one additional > commit since the last revision: > > 8292353: whitespace Never thought that I would ever feel on the verge of arguing against adding tests ;) > I haven't looked at these specific tests in detail, but as a general practice > I see value in having both types of tests: unit tests that test a specific > thing, and tests of various scenarios. Yeah, we could do additional testing of broader scenarios - assuming the tightly bounded specific tests are done. We should answer two questions: - what _exactly_ do we expect to gain, that is which additional safety do they give? - what is the boundary of those scenarios, why add one and not another? In this particular case we have a tighly bounded problem: the row updates itself incorrectly based on the state of the selection model. We know that all other potential collaborators talk to the selection model directly. Given we fixed the problem (done), and have failing/passing tests before/after the fix involving an isolated, fully configured cell (not yet done), my answers: - nothing I can see (might be missing something, of course :) - the choice of scenarios (scene only, simulating mouse input) feels arbitrary, why not key input (or touch, accessiblity .. )? They all are at the same level of unrelatedness, IMO - we are not testing the effectiveness of this bug fix, but something else (for the mouse input f.i. tableCell - behaviour) ------------- PR: https://git.openjdk.org/jfx/pull/875
