To write tests or not to write tests ...

[Kevin Rushforth]

In general, I would say it's OK to rely on existing tests for trivial 
refactoring, that is a refactoring where it seems fairly obvious that 
there are not going to be changes in behavior (even if such tests may be 
inadequate). For less trivial refactoring, if there are clearly missing 
tests, I would think they should be added to ensure no unintended 
behavioral changes, although I wouldn't go overboard.

Bug fixes and new features are a different matter. In the specific case 
you cite, there is new public API, so at the very least, testing the new 
API is needed. I haven't (yet) looked closely enough to know whether the 
refactoring is a possible source of concern that would warrant 
additional tests.

-- Kevin


On 11/8/2019 11:54 AM, Jeanette Winzenburg wrote:
>
> ... if changes are "just" a re-arrangement of code (like 
> https://github.com/openjdk/jfx/pull/6 - 8207957: TableSkinUtils should 
> not contain actual code implementation)?
>
> In an ideal world, there would be a safety-net of tests (they would 
> have been written at the time the old code was pushed - wouldn't they 
> ;). All we would need to do is run them to ensure that our 
> re-arrangement doesn't break anything.
>
> In the real world and that particular pull request above, there are 
> not tests (that I could find, maybe overlooked them).
>
> Now the question: is the contributor responsible for writing those 
> missing tests? The two extremes are
>
> - yes, it's a good opportunity ;) And it is mandatory (probably?) for 
> all public/protected api. A new protected method with specification 
> was condensed and should be tested.
> - no, nothing changed, the tests were always missing and all still 
> live ;) so doing nothing is just fine
>
> What's the procedure here?
>
> -- Jeanette
>