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
