Thanks for the guidance. Since my additional tests happen to test against a
regression that was actually happening I'll keep them in my PR. If the
reviewer finds them superfluous I can still remove them.
Am Di., 16. Juni 2020 um 16:17 Uhr schrieb Kevin Rushforth <
kevin.rushfo...@oracle.com>:
> 1. It's a judgment call. I tend to like fairly fine-grained tests as
> long as they are (at least mostly) independent. If the test is "do A",
> "verify state", "do B", "verify state" and B is something that you need
> to do after A, then having them in the same test is better.
>
> 2. It depends. If the additional tests are related to the area you are
> fixing and you are worried that the missing tests could lead to bugs if
> additional changes were made in the area you are fixing, then I think
> it's fine to add the new tests. If it's just a case where the test
> coverage is poor, it might make sense for a follow-on test bug to add
> new tests.
>
> I know that this isn't a definitive answer, but hopefully it will
> provide some general guidance.
>
> -- Kevin
>
>
> On 6/15/2020 10:21 PM, Robert Lichtenberger wrote:
> > While discussing my Pull Request ([1]) for JDK-8176270 ([2]) with
> Jeanette
> > Winzenburg I came across two questions that I would like to ask the
> > community regarding test cases:
> >
> > 1) How fine-grained to we want tests to be? Is it ok to test two
> (somewhat
> > similiar) things at once or should a separate test case be written? e.g.
> > this test case of mine:
> > @Test public void replaceSelectionAtEndWithListener() {
> > StringBuilder selectedTextLog = new StringBuilder();
> > StringBuilder selectionLog = new StringBuilder();
> > textInput.setText("x xxx");
> > textInput.selectRange(2, 5);
> > textInput.selectedTextProperty().addListener((__, ___,
> selection)
> > -> selectedTextLog.append("|" + selection));
> > textInput.selectionProperty().addListener((__, ___, selection)
> ->
> > selectionLog.append("|" + selection.getStart() + "," +
> selection.getEnd()));
> > textInput.replaceSelection("a");
> > assertEquals("|", selectedTextLog.toString());
> > assertEquals("|3,3", selectionLog.toString());
> > assertEquals("x a", textInput.getText());
> > }
> > Will test the selectedTextProperty AND the selectionProperty at the same
> > time. Is this acceptable/desireable or should the test case be split into
> > two separate tests?
> >
> > 2) When fixing bugs, is it ok to not only provide a test case that will
> > fail before the fix and run successfully after the fix but also provide
> > additional test cases with the intention of preventing regressions?
> Ideally
> > of course such tests should already exist but what if they are not.
> > In my case I wanted to add a test case to "prove" that redo() will work
> in
> > the presence of a shortened text and I would also like to have a general
> > test case about selection properties and text property. What's the
> general
> > rule here?
> >
> > Best regards,
> > Robert
> >
> > [1] https://github.com/openjdk/jfx/pull/138
> > [2] https://bugs.openjdk.java.net/browse/JDK-8176270
>
>
