Zitat von Kevin Rushforth <kevin.rushfo...@oracle.com>:

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.


reasonable middle ground :)

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.


in that specific case, I don't see much of a risk - and now there are tests for all aspects of the behavior as specified by the new api (as far as it can go, IMO, testing actual sizes in the context of column/cell/header is .. tricky to impossible)

-- Jeanette

-- 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




Reply via email to