Sometimes unit tests fail for reasons that are beyond the code that is
being tested. For instance, at work, we test code that uses a SQL
database. If the testing database server is down or misconfigured, the
tests fail even if the code is correct. Also, sometimes the tests fail
in a way that the solution is obvious.

In either of these cases, it can be frustrating to have to select
which changes to record and re-enter the patch name and comment. I
would like to be able to decide after the tests have failed whether or
not to record the patch. That way I can unrecord if it turns out the
patch is fundamentally flawed, or I can amend-record if the fix is
obvious.

Looks like you have a bad patch: My slightly flawed patch
Shall I record it anyway? [yN]

I think this is a happy medium between --no-test and flat-out
rejecting patches that don't pass the tests. I would be happy with
this behaviour being available through an option
(--ask-record-failed?) or on by default. I'm also willing to try to
implement it, if that helps.

Josh

Josh,

This should be fairly easy to implement.

Modify test_patch in Test.lhs so that it checks for
--ask-record-on-failed-test (or whatever sounds good).  Also look at
ArgumentDefault.lhs, DarcsArguments.lhs, DarcsCommand.lhs and
DarcsFlags.lhs to get a feel for how to implement the commandline
option.  I don't remember exactly which file the details are in, but
it should be in one or more of those.  If memory serves, you should
just need to add it to DarcsFlags for the record command and then
check that flag in test_patch.

Personally, I have no objection to a change like this, but I don't
think it should be the default behavior.  On the other hand, maybe  it
would be better if darcs remembered the details of the patch you
wanted to record when the test fails.  Then the next time you run
record it could take the unfinished patch out of pending and let you
try again (but being intelligent about updating the contents to
reflect any changes you made).

Good luck,
Jason

On 11/7/06, Josh Hoyt <bugs@darcs.net> wrote:
>
> New submission from Josh Hoyt <josh@janrain.com>:
>
> Sometimes unit tests fail for reasons that are beyond the code that is
> being tested. For instance, at work, we test code that uses a SQL
> database. If the testing database server is down or misconfigured, the
> tests fail even if the code is correct. Also, sometimes the tests fail
> in a way that the solution is obvious.
>
> In either of these cases, it can be frustrating to have to select
> which changes to record and re-enter the patch name and comment. I
> would like to be able to decide after the tests have failed whether or
> not to record the patch. That way I can unrecord if it turns out the
> patch is fundamentally flawed, or I can amend-record if the fix is
> obvious.
>
> Looks like you have a bad patch: My slightly flawed patch
> Shall I record it anyway? [yN]
>
> I think this is a happy medium between --no-test and flat-out
> rejecting patches that don't pass the tests. I would be happy with
> this behaviour being available through an option
> (--ask-record-failed?) or on by default. I'm also willing to try to
> implement it, if that helps.
>
> Josh
>
> ----------
> messages: 1178
> nosy: EricKow, droundy, josh, tommy
> status: unread
> title: Feature request: ask to record if tests fail
>
> ____________________________________
> Darcs issue tracker <bugs@darcs.net>
> <http://bugs.darcs.net/issue332>
> ____________________________________
>
> _______________________________________________
> darcs-devel mailing list
> darcs-devel@darcs.net
> http://www.abridgegame.org/cgi-bin/mailman/listinfo/darcs-devel
>
>

I agree this is a great feature to consider adding. Like "obliterate", this
feature is rooted in the idea the user knows best, and we should allow them to
do otherwise reckless things, as long they confirm that they know what they doing. 

I have myself been frustrated by tests which fail which are unrelated to the
patch I'm trying to record.

This feature is also something I've wanted many times as well--and especially
Jason's suggestion that darcs should remember the details of a patch that fails
a test, so that after you fix it you shouldn't have to enter all the information
again.

I'd be happy to take a look at implementing this sometime.  I could start by
implementing the --ask-record-failed feature etc, that seems easy enough. 
Anyone have any sense for how difficult it would be to get darcs to remember the
details of patches with failed tests, so they didn't have to be recreated from
scratch after fixing?

On Sun, Nov 02, 2008 at 03:55:04PM -0000, Brent Yorgey wrote:
> I'd be happy to take a look at implementing this sometime.  I could start by
> implementing the --ask-record-failed feature etc, that seems easy enough. 
> Anyone have any sense for how difficult it would be to get darcs to remember the
> details of patches with failed tests, so they didn't have to be recreated from
> scratch after fixing?

I suppose it depends on how precise you want to be.  By definition, you
aren't going to want to record the same set of patches as previously, so
which way do you want to err? Perhaps you want to ensure that any patches
that are identical to ones that previously were said 'no' to won't be
presented the second time? By the time you've decided what you want to do,
you'll probably know how hard it is.
-- 
David Roundy
http://www.darcs.net

On Sun, Nov 02, 2008 at 05:28:27PM -0000, David Roundy wrote:
>
> I suppose it depends on how precise you want to be.  By definition, you
> aren't going to want to record the same set of patches as previously, so

That's actually not true---in fact, many times when this happens (I
try to record a patch that fails) it is because of some external
dependency not being met.  So I update the external dependency and
then record the exact same patch, the difference being that this time
it is able to build.

> which way do you want to err? Perhaps you want to ensure that any patches
> that are identical to ones that previously were said 'no' to won't be
> presented the second time? By the time you've decided what you want to do,
> you'll probably know how hard it is.

How about this: any patches which are identical to patches from before
should not be presented a second time---they should remain in or out,
whatever was chosen the first time.  Only patches which are different
should be presented.  Also, the patch summary and optional long
comments should be preserved (and possibly available for editing).

How does that sound?  How difficult do you think this would be?  Also,
I'm completely fine with this not being the default.  Just the
possibility to turn this on would be great.  Maybe something like
darcs record --retry.

On Sun, Nov 02, 2008 at 05:28:27PM -0000, David Roundy wrote:
>
> I suppose it depends on how precise you want to be.  By definition, you
> aren't going to want to record the same set of patches as previously, so

That's actually not true---in fact, many times when this happens (I
try to record a patch that fails) it is because of some external
dependency not being met.  So I update the external dependency and
then record the exact same patch, the difference being that this time
it is able to build.

> which way do you want to err? Perhaps you want to ensure that any patches
> that are identical to ones that previously were said 'no' to won't be
> presented the second time? By the time you've decided what you want to do,
> you'll probably know how hard it is.

How about this: any patches which are identical to patches from before
should not be presented a second time---they should remain in or out,
whatever was chosen the first time.  Only patches which are different
should be presented.  Also, the patch summary and optional long
comments should be preserved (and possibly available for editing).

How does that sound?  How difficult do you think this would be?  Also,
I'm completely fine with this not being the default.  Just the
possibility to turn this on would be great.  Maybe something like
darcs record --retry.

This request seems motivated by a basic frustration of having to re-do your
patch selection after a record fails.  

Brent, Mark, Jason: it sounds like issue1052 (which proposes a mechanism by
which you could save your selection and then --retry) would cover some of the
stuff you've suggested in this ticket.  Please comment if this is not the case.

For what it's worth, I usually dislike adding confirmation prompts, but here I
think Josh's initial proposal of prompting when there is a test failure is the
simplest one, i.e. better than having extra flags.

Yes, issue1052 looks like it would cover what I want here.  For me at least,
you're correct that what I want isn't really the ability to record failing
patches per se, but to avoid redoing all the work to select the changes I want,
write a comment, etc. after fixing whatever was wrong.

I might be willing to try working on this at the hackathon in a few days--Eric,
you'll be there, right?

> This request seems motivated by a basic frustration of having to re-do your
> patch selection after a record fails.  
> 
> Brent, Mark, Jason: it sounds like issue1052 (which proposes a mechanism by
> which you could save your selection and then --retry) would cover some of the
> stuff you've suggested in this ticket.  Please comment if this is not the case.

I agree that Josh's original proposal is best here. To repeat it:

It allows you to be able to decide after the tests have failed whether or
not to record the patch. You could unrecord if it turns out the
patch is fundamentally flawed, or you could amend-record if the fix is
obvious. Example:

 Looks like you have a bad patch: My slightly flawed patch
 Shall I record it anyway? [yN]

> For what it's worth, I usually dislike adding confirmation prompts, but here I
> think Josh's initial proposal of prompting when there is a test failure is the
> simplest one, i.e. better than having extra flags.

Hopefully, the common cases are that either the tests all pass, or that you
give a flag not to run them at all (because you know for example you are
modifying documentation and not code). So, the extra confirmation prompt should
be rare, and hopefully a time-saver overall if you do want to record anyway
some of the time.

Eric, thanks for your work tending the bug tracker lately!

    Mark

The following patch sent by Gabriel Kerneis <kerneis@pps.jussieu.fr> updated issue issue332 with
status=resolved;resolvedin=2.8.0 HEAD

* resolve issue332: ask if test fails 
Ignore-this: 5d050e2e7716c81efa81010f91f75916