[Sugar-devel] Why automated testing and "dogfooding" is crucial (was: Re: code submitted for review should have been tested)
tomeu at sugarlabs.org
Wed Aug 18 08:04:40 EDT 2010
On Wed, Aug 18, 2010 at 11:54, Sascha Silbe
<sascha-ml-ui-sugar-devel at silbe.org> wrote:
> Excerpts from Tomeu Vizoso's message of Wed Aug 11 12:22:00 +0200 2010:
>> any code contributor is expecting that their patches will be tested by
>> the reviewer?
> A contributor should test their code to the best of their ability.
> A reviewer can, but doesn't have to test the code. A tester can, but
> doesn't have to review the code. With the mailing list based review model,
> this is expressed as the separate Reviewed-By and Tested-By tags.
>> Because of this specific commit, file transfers have been broken since
>> early this year and it's obvious that this code wasn't tested at all:
> Me being the original author of the patch, it's obvious that the code was
> tested rather well before submitting it for review. But
> a) I'm not perfect, so some code paths were not exercised before the
> initial submission for review
> b) it took many months before the patch was accepted, so it bitrotted.
> Simon probably had to solve a number of conflicts; also some changes
> in the surroundings of the patch might have been "ignored" by the VCS.
> All this is an error-prone process that requires any test to be
> re-run / re-done.
> This is a very nice example of
> a) how the long time patches wait in the review queue and the cumbersome
> process of submitting them to Trac is negatively affecting the project
> (I remember fixing an issue in this very patch related to file sharing,
> but probably never submitted an updated patch to Trac because it was
> rejected anyway back then).
> b) why automated testing is crucial. (I don't think I need to elaborate
> that point, it should be obvious from the above if it wasn't obvious
> before already.)
>> Given the current poor state of our testing efforts,
> The best way to get Sugar tested well (besides automatic tests which only
> catch specific cases) is by eating our own dog food. Of course, this
> requires us to accept some "high ceiling" patches (that no deployment
> will call for) instead of just "low floor" ones. This might increase the
> risk of breaking something (or raising the floor), but IMO is more than
> offset by the better testing and increased number of motivated
> contributors we get.
You make good points, but the bit about dogfooding doesn't hold as
well because I, for example, don't ever use file transfer in GNOME.
In any case, I don't think you are saying that if I propose a patch
that changes code in the file transfer path I shouldn't have tested
Btw, I've been looking at AT-SPI with eyes on accessibility and testing today.
> Please consider this mail food for thought, not a direct retort to your
> complaint about the breakage. That I was the one who authored this
> original patch is merely a detail showing that even careful and
> experienced contributors (as I consider myself to be) can't be relied upon
> to not make mistakes. We need to accept the fact that humans are imperfect
> and cater for it by tuning our processes and goals.
> Sugar-devel mailing list
> Sugar-devel at lists.sugarlabs.org
More information about the Sugar-devel