You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Erik Huelsmann <eh...@gmail.com> on 2005/10/26 13:01:44 UTC

Re: svn commit: r17032 - trunk/subversion/tests/clients/cmdline

On 10/26/05, lundblad@tigris.org <lu...@tigris.org> wrote:
> Author: lundblad
> Date: Wed Oct 26 07:41:19 2005
> New Revision: 17032
>
> Modified:
>   trunk/subversion/tests/clients/cmdline/revert_tests.py
>
> Log:
> * subversion/tests/client/cmdline/revert_tests.py
>  (test_list): Make revert_from_wc_root XFail.

Actually, no, that's not the purpose of XFail, as far as I conceive it:

Tests should be marked XFail if a feature currently isn't implemented
or behaving as it should, but that fact is recognised and agreed upon
[ie postponing a fix is acceptable].

This test however wasn't present when the wc-replacements branch was
merged, but the fact that it fails constitutes a regression. In other
words: it's absolutely not acceptable to XFail.

To ease the pain: I'll be fixing the problem tonight (probably), if
zhakov isn't working on it...

bye,


Erik.

Re: svn commit: r17032 - trunk/subversion/tests/clients/cmdline

Posted by Ivan Zhakov <ch...@gmail.com>.
On 10/26/05, Erik Huelsmann <eh...@gmail.com> wrote:
> > > To ease the pain: I'll be fixing the problem tonight (probably), if
> > > zhakov isn't working on it...
> > Sorry, I have personal problems and probably I cannot fix this issue
> > today. Any way  I already have thought on this issue.
>
> No problem, as I said: I'll try fixing it tonight.
Thanks, anyway I want to discuss your prop_path idea in IRC tonight.


--
Ivan Zhakov

Re: svn commit: r17032 - trunk/subversion/tests/clients/cmdline

Posted by Erik Huelsmann <eh...@gmail.com>.
> > To ease the pain: I'll be fixing the problem tonight (probably), if
> > zhakov isn't working on it...
> Sorry, I have personal problems and probably I cannot fix this issue
> today. Any way  I already have thought on this issue.

No problem, as I said: I'll try fixing it tonight.

bye,


Erik.

Re: svn commit: r17032 - trunk/subversion/tests/clients/cmdline

Posted by Ivan Zhakov <ch...@gmail.com>.
On 10/26/05, Erik Huelsmann <eh...@gmail.com> wrote:
> On 10/26/05, lundblad@tigris.org <lu...@tigris.org> wrote:
> > Author: lundblad
> > Date: Wed Oct 26 07:41:19 2005
> > New Revision: 17032
> >
> > Modified:
> >   trunk/subversion/tests/clients/cmdline/revert_tests.py
> >
> > Log:
> > * subversion/tests/client/cmdline/revert_tests.py
> >  (test_list): Make revert_from_wc_root XFail.
>
> Actually, no, that's not the purpose of XFail, as far as I conceive it:
>
> Tests should be marked XFail if a feature currently isn't implemented
> or behaving as it should, but that fact is recognised and agreed upon
> [ie postponing a fix is acceptable].
>
> This test however wasn't present when the wc-replacements branch was
> merged, but the fact that it fails constitutes a regression. In other
> words: it's absolutely not acceptable to XFail.
>
> To ease the pain: I'll be fixing the problem tonight (probably), if
> zhakov isn't working on it...
Sorry, I have personal problems and probably I cannot fix this issue
today. Any way  I already have thought on this issue.

--
Ivan Zhakov

Re: When to commit failing tests (was: Re: svn commit: r17032 - trunk/subversion/tests/clients/cmdline)

Posted by Erik Huelsmann <eh...@gmail.com>.
[ snip about creating issues for XFAIL tests ]

> I am +1 on Peter's point of view. Also we can add rule that any XFail
> test should have issue. And keep issue number written near XFail test?
> It prevent forget bugs.

ok, ok, if that's how we work, I'll comply...


bye,


Erik.

Re: When to commit failing tests (was: Re: svn commit: r17032 - trunk/subversion/tests/clients/cmdline)

Posted by Ivan Zhakov <ch...@gmail.com>.
On 26 Oct 2005 12:43:57 -0500, kfogel@collab.net <kf...@collab.net> wrote:
> "Peter N. Lundblad" <pe...@famlundblad.se> writes:
> > You normally don't commit something that you know have failing tests. If
> > the nightly build fail, because you did something platform-dependent or
> > something, then that's another story. I really don't think knowingly
> > commit failing tests non-XFail is a good way of alerting people that there
> > is a regression. You can post to dev@, file an issue or just fix it in a
> > few days.
> >
> > The problem with your approach is that it slows down things for everyone
> > else. When I want to run tests and get a failure, I have to examine and
> > dscover that it was just that other regression that still fails. If I made
> > a commit, I have to examine the nightly failure mails, because I can't
> > just assume that it is your test that fail. I think that's a waste of
> > time.
>
> +1 IMHO.
>
> The value of the test suite and the nightly builds is in notifying us
> of things we don't already know.  So if someone knows that a test is
> failing, then we don't need to get that information by other means --
> the person can just set the test XFail, and file an issue or do
> whatever is appropriate, to record the fact of the failure.
I am +1 on Peter's point of view. Also we can add rule that any XFail
test should have issue. And keep issue number written near XFail test?
It prevent forget bugs.

--
Ivan Zhakov

Re: When to commit failing tests (was: Re: svn commit: r17032 - trunk/subversion/tests/clients/cmdline)

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 26 Oct 2005, Justin Erenkrantz wrote:

> --On October 26, 2005 12:43:57 PM -0500 kfogel@collab.net wrote:
>
> > the person can just set the test XFail, and file an issue or do
>
> I don't have as strong a problem with setting it to XFail (could we add a
> new code such as KnownFailure?), but not committing the test to the tree
> because there's no fix is my concern.  -- justin
>
Then it seems like we are in agreement. My original complaint (or rather
commit) was to make a failing test XFail. I'm *for* XFail tests.

regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: When to commit failing tests (was: Re: svn commit: r17032 - trunk/subversion/tests/clients/cmdline)

Posted by kf...@collab.net.
Justin Erenkrantz <ju...@erenkrantz.com> writes:
> I don't have as strong a problem with setting it to XFail (could we
> add a new code such as KnownFailure?), but not committing the test to
> the tree because there's no fix is my concern.  -- justin

This discussion is about committing a failing test not as XFail; as
far as I know there is no other controversy here...

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: When to commit failing tests (was: Re: svn commit: r17032 - trunk/subversion/tests/clients/cmdline)

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On October 26, 2005 12:43:57 PM -0500 kfogel@collab.net wrote:

> the person can just set the test XFail, and file an issue or do

I don't have as strong a problem with setting it to XFail (could we add a 
new code such as KnownFailure?), but not committing the test to the tree 
because there's no fix is my concern.  -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: When to commit failing tests (was: Re: svn commit: r17032 - trunk/subversion/tests/clients/cmdline)

Posted by kf...@collab.net.
"Peter N. Lundblad" <pe...@famlundblad.se> writes:
> You normally don't commit something that you know have failing tests. If
> the nightly build fail, because you did something platform-dependent or
> something, then that's another story. I really don't think knowingly
> commit failing tests non-XFail is a good way of alerting people that there
> is a regression. You can post to dev@, file an issue or just fix it in a
> few days.
> 
> The problem with your approach is that it slows down things for everyone
> else. When I want to run tests and get a failure, I have to examine and
> dscover that it was just that other regression that still fails. If I made
> a commit, I have to examine the nightly failure mails, because I can't
> just assume that it is your test that fail. I think that's a waste of
> time.

+1 IMHO.

The value of the test suite and the nightly builds is in notifying us
of things we don't already know.  So if someone knows that a test is
failing, then we don't need to get that information by other means --
the person can just set the test XFail, and file an issue or do
whatever is appropriate, to record the fact of the failure.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: When to commit failing tests (was: Re: svn commit: r17032 - trunk/subversion/tests/clients/cmdline)

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 26 Oct 2005, Justin Erenkrantz wrote:

> --On October 26, 2005 8:24:45 PM +0200 "Peter N. Lundblad"
> <pe...@famlundblad.se> wrote:
>
> > The problem with your approach is that it slows down things for everyone
> > else. When I want to run tests and get a failure, I have to examine and
> > dscover that it was just that other regression that still fails. If I made
> > a commit, I have to examine the nightly failure mails, because I can't
> > just assume that it is your test that fail. I think that's a waste of
> > time.
>
> If you're annoyed by the failing test, then you can fix the bug.  -- justin
>
Crap! Fixing 1.4 regressions has far less priority than fixing, say, an
1.3.x problem.  And I don't like to be "forced" to fix particular bugs.

Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: When to commit failing tests (was: Re: svn commit: r17032 - trunk/subversion/tests/clients/cmdline)

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On October 26, 2005 8:24:45 PM +0200 "Peter N. Lundblad" 
<pe...@famlundblad.se> wrote:

> The problem with your approach is that it slows down things for everyone
> else. When I want to run tests and get a failure, I have to examine and
> dscover that it was just that other regression that still fails. If I made
> a commit, I have to examine the nightly failure mails, because I can't
> just assume that it is your test that fail. I think that's a waste of
> time.

I disagree.  If we have a known regression and a test that indicates the 
bug, we should check it in as soon as possible - even if we don't have a 
fix at hand.

If you're annoyed by the failing test, then you can fix the bug.  -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

When to commit failing tests (was: Re: svn commit: r17032 - trunk/subversion/tests/clients/cmdline)

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
Hi,

On Wed, 26 Oct 2005, Erik Huelsmann wrote:

> > On 10/26/05, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> > On Wed, 26 Oct 2005, Erik Huelsmann wrote:
> >
> > >> On 10/26/05, lundblad@tigris.org <lu...@tigris.org> wrote:
> > >> Log:
> > >> * subversion/tests/client/cmdline/revert_tests.py
> > >>  (test_list): Make revert_from_wc_root XFail.
> >
> > > Actually, no, that's not the purpose of XFail, as far as I conceive
> it:
>
> > > Tests should be marked XFail if a feature currently isn't
> implemented
> > > or behaving as it should, but that fact is recognised and agreed
upon
> > > [ie postponing a fix is acceptable].
>
> > This has come up before. Yes, it is a regression that needs to be
> fixed.
> > But what's the point in making all nightly builds fail and all others
> > tests fail? Either you just fix it, or add an issue as a 1.4.0
> blocker.

> Well, the point being that if there's a regression, normally, failing
> tests indicate it. In this case, there hasn't been the same
> indication. Other devs would have missed it if they weren't on irc
> yesterday. Also, I think there's a difference between a once correctly
> implemented behaviour not working anymore and a newly implemented
> feature not working entirely as expected.

You normally don't commit something that you know have failing tests. If
the nightly build fail, because you did something platform-dependent or
something, then that's another story. I really don't think knowingly
commit failing tests non-XFail is a good way of alerting people that there
is a regression. You can post to dev@, file an issue or just fix it in a
few days.

The problem with your approach is that it slows down things for everyone
else. When I want to run tests and get a failure, I have to examine and
dscover that it was just that other regression that still fails. If I made
a commit, I have to examine the nightly failure mails, because I can't
just assume that it is your test that fail. I think that's a waste of
time.

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r17032 - trunk/subversion/tests/clients/cmdline

Posted by Erik Huelsmann <eh...@gmail.com>.
On 10/26/05, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> On Wed, 26 Oct 2005, Erik Huelsmann wrote:
>
> >> On 10/26/05, lundblad@tigris.org <lu...@tigris.org> wrote:
> >> Log:
> >> * subversion/tests/client/cmdline/revert_tests.py
> >>  (test_list): Make revert_from_wc_root XFail.
>
> > Actually, no, that's not the purpose of XFail, as far as I conceive it:
>
> > Tests should be marked XFail if a feature currently isn't implemented
> > or behaving as it should, but that fact is recognised and agreed upon
> > [ie postponing a fix is acceptable].
>
> This has come up before. Yes, it is a regression that needs to be fixed.
> But what's the point in making all nightly builds fail and all others
> tests fail? Either you just fix it, or add an issue as a 1.4.0 blocker.

Well, the point being that if there's a regression, normally, failing
tests indicate it. In this case, there hasn't been the same
indication. Other devs would have missed it if they weren't on irc
yesterday. Also, I think there's a difference between a once correctly
implemented behaviour not working anymore and a newly implemented
feature not working entirely as expected.

> Maybe others disagree with this opinion, and then I'll have to regret, I
> guess:-)

No need to regret. It's good to discuss these things every now and
then. Hopefully we reach concensus and document in hacking.html?

Anyway: I'll make sure it's unmarked XFail tonight (because it'll be
fixed by then).

bye,

Erik.

Re: svn commit: r17032 - trunk/subversion/tests/clients/cmdline

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 26 Oct 2005, Erik Huelsmann wrote:

>> On 10/26/05, lundblad@tigris.org <lu...@tigris.org> wrote:
>> Log:
>> * subversion/tests/client/cmdline/revert_tests.py
>>  (test_list): Make revert_from_wc_root XFail.

> Actually, no, that's not the purpose of XFail, as far as I conceive it:

> Tests should be marked XFail if a feature currently isn't implemented
> or behaving as it should, but that fact is recognised and agreed upon
> [ie postponing a fix is acceptable].

This has come up before. Yes, it is a regression that needs to be fixed.
But what's the point in making all nightly builds fail and all others
tests fail? Either you just fix it, or add an issue as a 1.4.0 blocker.

Maybe others disagree with this opinion, and then I'll have to regret, I
guess:-)

Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org