[PATCH 1 of 2 stable] fileset: add tests of generated working copy states

[Martin von Zweigbergk]

On Fri Jan 23 2015 at 4:08:34 AM Yuya Nishihara <yuya at tcha.org> wrote:

> On Thu, 22 Jan 2015 14:21:27 -0800, Martin von Zweigbergk wrote:
> > # HG changeset patch
> > # User Martin von Zweigbergk <martinvonz at google.com>
> > # Date 1421883624 28800
> > #      Wed Jan 21 15:40:24 2015 -0800
> > # Branch stable
> > # Node ID 9e6464357f9dc8f145e30b0af89e3deb1bc5c2dd
> > # Parent  a43fdf33a6beb697945a3dbb7253f0436ea278a6
> > fileset: add tests of generated working copy states
>
> > +Test revert
> > +
> > +BROKEN: the files that get undeleted were not modified, they were
> removed,
> > +and content1_content2_missing-tracked was also not modified, it was
> deleted
> > +
> > +  $ hg revert 'set:modified()'
> > +  reverting content1_content1_content3-tracked
> > +  reverting content1_content2_content1-tracked
> > +  undeleting content1_content2_content1-untracked
> > +  undeleting content1_content2_content2-untracked
> > +  reverting content1_content2_content3-tracked
> > +  undeleting content1_content2_content3-untracked
> > +  reverting content1_content2_missing-tracked
> > +  undeleting content1_content2_missing-untracked
> > +  reverting missing_content2_content3-tracked
> > +
> > +BROKEN: only the files that get forgotten are correct
> > +
> > +  $ hg revert 'set:added()'
> > +  forgetting content1_missing_content1-tracked
> > +  forgetting content1_missing_content3-tracked
> > +  undeleting missing_content2_content2-untracked
> > +  undeleting missing_content2_content3-untracked
> > +  reverting missing_content2_missing-tracked
> > +  undeleting missing_content2_missing-untracked
> > +  forgetting missing_missing_content3-tracked
> > +
> > +  $ hg revert 'set:removed()'
> > +  undeleting content1_content1_content1-untracked
> > +  undeleting content1_content1_content3-untracked
> > +  undeleting content1_content1_missing-untracked
> > +
> > +  $ hg revert 'set:deleted()'
> > +  reverting content1_content1_missing-tracked
> > +  forgetting content1_missing_missing-tracked
> > +  forgetting missing_missing_missing-tracked
> > +
> > +  $ hg revert 'set:unknown()'
> > +
> > +  $ hg revert 'set:clean()'
>
> "revert" tests will have mostly undesired result due of issue4497, so I
> don't
> think it's worth adding them at this point.
>
> http://bz.selenic.com/show_bug.cgi?id=4497


That issue is what made look into this. I started writing these tests to
document what's broken and to make sure the tests caught the broken
behavior. I think you're not alone in your dislike of adding broken tests,
though. I believe mpm also dislikes them. I'll explain the value I see in
adding broken tests to get the debate started:

 * If the tests are written and I don't have time or motivation at the
moment to fix the issue, then at least they're there for the next person
who works on the issue. (In this particular case, my goal was to fix issue
4497, but I don't want to promise I will fix, and if I don't end up fixing
it, it seems like a waste to throw away the tests.)

 * It makes it clear what the change in behavior is as we add a fix. And
that the test case catches it. In fact, when tests are added in the same
changeset as the fix, I often wonder what the output would have been
without the fix. I wonder both because I'm trying to understand the effect
of the patch, and because I want to make sure the test case would have
caught the bug (i.e. that it fails before the fix).

The disadvantages I can see:

* It's easy to leave a stray "BROKEN" marker after fixing the issue. This
is a little annoying. I don't know if some infrastructure could help with
it.

* Maybe what you think is broken is the way it's supposed to work.
Hopefully the reviewers would catch this, but if not, it's just a matter of
dropping the "BROKEN" once people agree that it's working as intended.

* It bloats the repository. I'm hoping that non-binary changes are
considered cheap enough that this is not really an issue.

Martin
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150123/704bf405/attachment.html>