On Tue, Mar 10, 2009 at 14:55, Stefan Sperling <stsp_at_elego.de> wrote:
>...
> I concur. What's wrong with a test that occasionally fails if it is
> known to occasionally fail because a bug has not been found yet?
> As long as people know that this particular test is not failing
> because of their own changes, that's fine.
Red lights on the buildbots every single day helps nobody.
Sure, *you* can ignore svnpatch_tests.py output. But the buildbots do not.
> The point of the test suite shouldn't be "all tests must pass all
> the time no matter what".
Yes, actually it SHOULD.
The buildbots help us to identify problems. If tests are left broken,
then I make changes and run the test suite, and get broken tests... is
it my fault? Or were they broken before? I will have to dig in to be
sure.
Or, worse: I knew they were failing before, so I ignore them. However,
somebody goes to fix the test, but now it *really* fails because of
the change that I made. The person fixing the test is going to have to
dig up what I did, figure it out, and then figure out how to fix the
problem I introduced. Or maybe it gets punted back to me two weeks
later. *Then* I realize I made a horrible mistake and should never
have done the change in the first place, but how was I to know? I
didn't have the test to tell me.
So. Trunk should ALWAYS build, and the ENTIRE test suite should ALWAYS pass.
Consider last fall when Karl was doing some work. He merged in a
branch and found the test suite failing. He banged his head against
the wall for several hours trying to discover the problem. THEN
somebody told him, "oh yeah. those tests have been failing for the
past week." Do you realize how much developer time and effort is
wasted by allowing the test suite to have failing tests?
It is possible that you check something in, and it breaks some tests
and you don't realize it. But it'll pop up on the buildbots "soon" and
somebody will let you know. You can then go fix the problem, and run
those whole test suite until it passes again.
> If a test is known to fail because of a particular commit, and we
> don't want to annoy people with a failing test, we could back out
> the commit and the test, and commit a working combination once the
> problem has been found. I've done this before.
No no no. "revert" is not the answer. *Moving forward* is the answer.
In this case, all that code stays in there, and Bert simply disabled
the tests for now.
We all get to review the code on trunk, make improvements and fixes,
and hey... maybe somebody will dig in and fix the tests. But if you
backed out the change? Then NOBODY can work on the problems.
Now... should that branch have been merged with failing tests?
Probably not. But they all worked on Arfrever's box. He didn't know
they'd fail on MacOS or Windows. That's an allowable situation because
we cannot and do not expect our developers to be able to run tests on
all platforms. So the merge may have been a bit premature, but it
wasn't *wrong* for Arfrever to merge it.
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1303223
Received on 2009-03-10 15:11:37 CET