On Mon, 16 Mar 2020 13:00:32 GMT, Jeanette Winzenburg <[email protected]> 
wrote:

>> Good catch!!!
>> It definitely would have unintentional side effect of incorrect orientation 
>> in subsequent tests.
>> I updated the test to restore the orientation at the end of the test. Also 
>> documented the same.
>
> hmm ... don't think that it will effect subsequent tests, though might be 
> mis-understanding the api doc of
> Parameterized:
>> When running a parameterized test class, instances are created for the 
>> cross-product of the test methods and the test
>> data elements.
> 
> So just suggested to change the doc of the method, something like
> 
>     Changes the parameter nodeOrientation and sets the table's orientation to 
> the new value
> 
> Alternatively, leave the parameter alone, just toggle the table's orientation 
> and change the forward/backward methods
> to choose the keys based on the table's current orientation. Or fully 
> parameterize on the keys - sry, can't resist :)

Thanks for clarifying my impression about side effect on subsequent tests.
I have the updated the method documentation (also changed the method name to 
toggleNodeOrientation() to clearly reflect
what it does)

There would always be different ways to test the same thing. I think, current 
test is in good shape and suffices the
purpose of unit testing this fix.

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

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

Reply via email to