[openstack-dev] When is it okay for submitters to say 'I don't want to add tests' ?

Christopher Yeoh cbkyeoh at gmail.com
Thu Oct 31 03:37:04 UTC 2013


> When is it okay for submitters to say 'I don't want to add tests' ?

When they don't want their patch merged :-) But more seriously....

On Thu, Oct 31, 2013 at 1:07 PM, Robert Collins
<robertc at robertcollins.net>wrote:

> This is a bit of a social norms thread....
>
> I've been consistently asking for tests in reviews for a while now,
> and I get the occasional push-back. I think this falls into a few
> broad camps:
>
> A - there is no test suite at all, adding one in unreasonable
>

Do you mean no test suite at all for the specific project or the feature
they are modifying?
For the latter I think its fine to require that they start one for the
feature even if it just tests
the behavior they just modified.


> B - this thing cannot be tested in this context (e.g. functional tests
> are defined in a different tree)
> C - this particular thing is very hard to test
>

Yea I have a lot of sympathy for this one. I run into this occasionally
myself
where it can be really difficult to cleanly, yet still vaguely
realistically inject
an error. I'm sort of stuck on one at the moment and I've definitely spent
a lot more
time thinking about how to write an adequate test than the fix itself.


> D - testing this won't offer benefit
>

The test will at least always act as a check that the specific behaviour
doesn't change
accidentally in the future won't it (in the context of bug fixes) ?



> E - other things like this in the project don't have tests
> F - submitter doesn't know how to write tests
> G - submitter doesn't have time to write tests
>
>
There is perhaps also:

H - a test in another project (say tempest) picks up this failure already.

and H is in combination with say C. I'd have a lot of sympathy for that
situation.


Now, of these, I think it's fine not add tests in cases A, B, C in
> combination with D, and D.
>
> I don't think E, F or G are sufficient reasons to merge something
> without tests, when reviewers are asking for them. G in the special
> case that the project really wants the patch landed - but then I'd
> expect reviewers to not ask for tests or to volunteer that they might
> be optional.
>

I agree with you on E, F and G. But in those circumstances we should
be offering those people help in writing tests, especially if they're new.

Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20131031/833268fa/attachment.html>


More information about the OpenStack-dev mailing list