openstack team mailing list archive

[Ionuț Arțăriși <iartarisi@xxxxxxx>]

Hi Mark, thanks for your answer.

On 05/03/2012 10:25 AM, Mark McLoughlin wrote:
> Hi,
>
> On Wed, 2012-05-02 at 14:37 +0200, Ionuț Arțăriși wrote:
> > I recently submitted a few fixes to the test suite in various components
> > of openstack.
>
> Thanks for that!
>
>
> > These fixes are being merged in master, but the code remains broken in
> > the stable/essex branch. Review requests for  stable/essex either get
> > rejected or stuck in limbo because it seems that people don't know
> > what to do about them.
>
> We're talking about this?
>
>    https://review.openstack.org/#/c/6619/

and this: https://bugs.launchpad.net/keystone/+bug/983800/ (comment #2)


> The issue here is that "Swift is different" :)
>
> Swift's releases are intended to be stable updates and AFAIK the Swift
> team feels that they don't need to do separate stable update releases.
> So the stable branch process we've put in place for the other projects
> doesn't apply to Swift.
>
> I've added a note to the wiki page about this.

Ok, good to know. Thanks.

> > I am aware of the procedure for backporting fixes[1], but I think it
> > does not deal with this issue correctly.
> >
> > Fixes to test scripts are for our benefit, the developers'. They don't
> > affect the users in any way. I don't think test code should be thrown
> > together with application code when thinking about making changes to
> > it.
> >
> > Only making test changes to the master branches reflects a belief that
> > tests are only used during development. I don't think this is
> > true. Tests, especially functional tests, are also incredibly useful
> > during maintenance. e.g. They help us test against different library
> > versions/distro than the one that's used for development and using
> > different deployment configurations.
> >
> > I suspect we're not the only downstream running the various testsuites
> > against their own packaged versions of different openstack
> > branches. Backporting these changes not only spares the time of other
> > projects who might run into these bugs on the stable branches later, it
> > also gives all of us the benefit of not having to fork the project just
> > so we can attach our patches. OTOH blocking test backports removes the
> > incentive that downstream projects have for reporting those bugs and
> > sending fixes for them upstream.
> >
> > So can we talk about separating the tests from the application code at
> > least as far as the backports are concerned? What about having the
> > 'test/' directory as a git submodule?
> >
> > Or maybe I don't understand this problem enough. What are the downsides
> > to backporting test-only fixes? Do they really outweigh the advantages?
>
> I think you've misinterpreted the response to your review so I won't go
> into the specifics of your points, but here's how I think about the
> backporting of unit tests:
>
>    - Unit test's main value IMHO is preventing regressions during
>      development.
>
>      Adding new unit tests can occasionally find new bugs too but, if I
>      want to find bugs on a stable branch, it would be functional tests
>      I'd write.
>
>    - As such, I don't think a concerted effort across the project to
>      systematically backport unit tests from master to the stable branch
>      is time well spent.

I'm not advocating that someone spend time to look through all the 
patches to unittests in master and backport them to stable. What I'm 
asking for is that when someone submits a test fix backport it would be 
*accepted* in a stable branch after the proper review process.


>      However, anyone is welcome to maintain their own tree based off the
>      stable branch and use that as a venue for finding bugs with new
>      unit tests. I'd hope those tests would go into master first, though.
>
>    - Also, a worthwhile goal on a stable branch is to keep churn to a
>      minimum. You could argue that tests deserve a free pass because
>      changes to those can't introduce regressions but, when it comes to
>      a stable branch, I'm sceptical that any patch is "zero risk" never
>      mind a whole class of (generally quite large) patches.

I think you're again talking about backporting all test fixes here in 
bulk. But apart from that, I don't really see how testsuite-only patches 
which go through the same review process as normal patches can break 
anything in application code and be deemed "risky"?


>      Part of keeping churn to a minimum on a stable branch is that
>      reviewers default answer should be "no". Unless a patch meets the
>      "safe fix for a high-impact, user-visible issue" then it doesn't
>      belong in the stable branch.
>
>    - That said, if someone adds a new test to master and it uncovers a
>      significant user-visible bug (or uncovers a bug and adds a test for
>      it), then I think it makes sense to backport the unit test to master
>      along with the bug fix. It helps verify that the backported patch
>      does fix the bug.
>
>    - And finally, I think it's sane for downstreams to run the unit
>      tests. As you say, it can catch issues where downstream is using a
>      different version of a library than upstream. If a downstream
>      uncovers an issue like this, fixes it on master with a unit test
>      and backports the fix and unit test to stable, I think that's great
>      too.

Well you think it's great and I think it's great, but somehow we don't 
agree with one another? This is what I did. Uncovered a bug in the tests 
on a different version/configuration, submitted the fix to master -> it 
got accepted. Submitted the fix to stable -> it got rejected. Or are you 
saying that these fixes are not important because they're only testsuite 
fixes?

Finally, I'm fine with any attitude to backporting test fixes (though I 
strongly prefer backporting them), but let's please make it deliberate 
and clear and then document it on the wiki page. This is why I started 
this discussion.

-Ionuț