One approach for sometimes avoiding test changes in fix patches

[Daniel John Debrunner <dj...@apache.org>]

Knut wrote in the patch submission for DERBY-822

> I hope the size of the patch doesn't scare off potential
> reviewers. The actual code changes are very small. 95% of the patch is
> just updating the master files for the Wisconsin test.

I thought I would share my approach to this issue. This approach works
when improving the code while not adding any user visible features (as
Knut is doing). It is also applicable when implementing a feature
requires additional testing of the existing functionality, because, say,
you notice some areas are not tested.

I skimmed Knut's description of the patch and several of the changes are
related to changing the tests to ensure they could provide consistent
behaviour in embedded and client, now with pre-fetch. These changes
seemed to be mainly actually execute the queries and fetch the data,
rather than just execute with no fetch.

Thus these test changes can be made without requiring Knut's code
changes for pre-fetch, thus they are independent. With test changes like
this I make the test changes in a separate clean codeline and not in my
development codeline. Then I commit the change from the clean codeline
and do a svn update to bring them into the development line.
And it's an identical process if while I'm improving an area I find the
existing functionality could do with more testing, then I make test
improvements in the clean code line.

This is a little more work but I find it has these benefits:

  - It gives me, the coder, more confidence that my real code change is
correct, especially when re-writing exiting functionality without
changing the user visible features. With a combined test and code
checkin, can I be sure I'm not changing behaviour, maybe something I
throw an error for now, correctly worked before my change? With separate
test & code checkins that worry goes away, because the tests are
committed against the existing code before my code changes. And you can
let them run against the nightlies for a couple of days before
committing the code changes.

  - Additional testing patches are easier for reviewers, since they can
see the product functionality is not affected, it's just additional
testing and that's always a good thing.

   - Reviewing the final code patch is easier, focus on the actually
code changes and not lines of test changes. Less chance of missing some
change in the tests that is incorrect.

Just my approach,
Dan.

Re: One approach for sometimes avoiding test changes in fix patches

[Knut Anders Hatlen <Kn...@Sun.COM>]

Daniel John Debrunner <dj...@apache.org> writes:

> Knut wrote in the patch submission for DERBY-822
>
>> I hope the size of the patch doesn't scare off potential
>> reviewers. The actual code changes are very small. 95% of the patch is
>> just updating the master files for the Wisconsin test.
>
> I thought I would share my approach to this issue. This approach works
> when improving the code while not adding any user visible features (as
> Knut is doing). It is also applicable when implementing a feature
> requires additional testing of the existing functionality, because, say,
> you notice some areas are not tested.
>
> I skimmed Knut's description of the patch and several of the changes are
> related to changing the tests to ensure they could provide consistent
> behaviour in embedded and client, now with pre-fetch. These changes
> seemed to be mainly actually execute the queries and fetch the data,
> rather than just execute with no fetch.
>
> Thus these test changes can be made without requiring Knut's code
> changes for pre-fetch, thus they are independent. With test changes like
> this I make the test changes in a separate clean codeline and not in my
> development codeline. Then I commit the change from the clean codeline
> and do a svn update to bring them into the development line.
> And it's an identical process if while I'm improving an area I find the
> existing functionality could do with more testing, then I make test
> improvements in the clean code line.

Hi Dan,

Thanks for the advice! I really like that approach. I'll extract those
changes I have made to the tests that are independent of the
pre-fetching and submit them as a separate patch. In the patch I
submitted, most of the test changes were made to the canons only,
hence they are not independent of the code changes. However, I think
most of the tests can be modified so that they have the same behaviour
regardless of pre-fetching. If I make those changes to the tests, the
code changes are no longer indirectly tested by the modified tests, so
it might be a good idea to add some explicit regression tests as well.

-- 
Knut Anders