Patchwork [1,of,3,STABLE,v2] tests: add test for failing bookmark push code

login
register
mail settings
Submitter Gregory Szorc
Date Oct. 24, 2014, 6:07 p.m.
Message ID <8344ea64087d5eaab427.1414174078@3.1.168.192.in-addr.arpa>
Download mbox | patch
Permalink /patch/6461/
State Accepted
Headers show

Comments

Gregory Szorc - Oct. 24, 2014, 6:07 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1414171261 25200
#      Fri Oct 24 10:21:01 2014 -0700
# Branch stable
# Node ID 8344ea64087d5eaab42709aaab6daa38dbde3db8
# Parent  eb763217152ab2b472416bcc57722451c317f282
tests: add test for failing bookmark push code

b901645a8784 regressed the behavior of pushing an unchanged bookmark to
a remote. Before that commit, pushing a unchanged bookmark would result
in "exporting bookmark @" being printed. After that commit, we now see
an incorrect message "bookmark %s does not exist on the local or remote
repository!"

This patch adds a test that demonstrates the faulty behavior.
Pierre-Yves David - Oct. 24, 2014, 10:41 p.m.
On 10/24/2014 08:07 PM, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1414171261 25200
> #      Fri Oct 24 10:21:01 2014 -0700
> # Branch stable
> # Node ID 8344ea64087d5eaab42709aaab6daa38dbde3db8
> # Parent  eb763217152ab2b472416bcc57722451c317f282
> tests: add test for failing bookmark push code
>
> b901645a8784 regressed the behavior of pushing an unchanged bookmark to
> a remote. Before that commit, pushing a unchanged bookmark would result
> in "exporting bookmark @" being printed. After that commit, we now see
> an incorrect message "bookmark %s does not exist on the local or remote
> repository!"

I would argue that the "exporting bookmark X" is from if no change is 
actually made to the bookmark.


> This patch adds a test that demonstrates the faulty behavior.
>
> diff --git a/tests/test-bookmarks-pushpull.t b/tests/test-bookmarks-pushpull.t
> --- a/tests/test-bookmarks-pushpull.t
> +++ b/tests/test-bookmarks-pushpull.t
> @@ -437,4 +437,32 @@ pushing a new bookmark on a new head doe
>     $ hg -R ../b id -r W
>     cc978a373a53 tip W
>
>     $ cd ..
> +
> +pushing an unchanged bookmark should result in no changes
> +
> +  $ hg init unchanged-a
> +  $ hg init unchanged-b
> +  $ cd unchanged-a
> +  $ echo initial > foo
> +  $ hg commit -A -m initial
> +  adding foo
> +  $ hg bookmark @
> +  $ hg push -B @ ../unchanged-b
> +  pushing to ../unchanged-b
> +  searching for changes
> +  adding changesets
> +  adding manifests
> +  adding file changes
> +  added 1 changesets with 1 changes to 1 files
> +  exporting bookmark @
> +
> +BUGGY We should simply say "no changes found"
> +  $ hg push -B @ ../unchanged-b
> +  pushing to ../unchanged-b
> +  searching for changes
> +  bookmark @ does not exist on the local or remote repository!
> +  no changes found
> +  [2]
> +
> +  $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Gregory Szorc - Oct. 24, 2014, 11:19 p.m.
On 10/24/14 3:41 PM, Pierre-Yves David wrote:
>
>
> On 10/24/2014 08:07 PM, Gregory Szorc wrote:
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc@gmail.com>
>> # Date 1414171261 25200
>> #      Fri Oct 24 10:21:01 2014 -0700
>> # Branch stable
>> # Node ID 8344ea64087d5eaab42709aaab6daa38dbde3db8
>> # Parent  eb763217152ab2b472416bcc57722451c317f282
>> tests: add test for failing bookmark push code
>>
>> b901645a8784 regressed the behavior of pushing an unchanged bookmark to
>> a remote. Before that commit, pushing a unchanged bookmark would result
>> in "exporting bookmark @" being printed. After that commit, we now see
>> an incorrect message "bookmark %s does not exist on the local or remote
>> repository!"
>
> I would argue that the "exporting bookmark X" is from if no change is
> actually made to the bookmark.

I considered this. But I wanted to write a backward-compatible patch to 
minimize contention and increase chances of making 3.2.
Pierre-Yves David - Oct. 26, 2014, 10:55 a.m.
On 10/25/2014 01:19 AM, Gregory Szorc wrote:
> On 10/24/14 3:41 PM, Pierre-Yves David wrote:
>>
>>
>> On 10/24/2014 08:07 PM, Gregory Szorc wrote:
>>> # HG changeset patch
>>> # User Gregory Szorc <gregory.szorc@gmail.com>
>>> # Date 1414171261 25200
>>> #      Fri Oct 24 10:21:01 2014 -0700
>>> # Branch stable
>>> # Node ID 8344ea64087d5eaab42709aaab6daa38dbde3db8
>>> # Parent  eb763217152ab2b472416bcc57722451c317f282
>>> tests: add test for failing bookmark push code
>>>
>>> b901645a8784 regressed the behavior of pushing an unchanged bookmark to
>>> a remote. Before that commit, pushing a unchanged bookmark would result
>>> in "exporting bookmark @" being printed. After that commit, we now see
>>> an incorrect message "bookmark %s does not exist on the local or remote
>>> repository!"
>>
>> I would argue that the "exporting bookmark X" is from if no change is
>> actually made to the bookmark.
>
> I considered this. But I wanted to write a backward-compatible patch to
> minimize contention and increase chances of making 3.2.

I would argue that this "exporting bookmark @" when nothing is actually 
moved is a bug and worthy of fixing on stable.
Augie Fackler - Oct. 27, 2014, 1:10 p.m.
On Oct 26, 2014, at 6:55 AM, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:

> 
> 
> On 10/25/2014 01:19 AM, Gregory Szorc wrote:
>> On 10/24/14 3:41 PM, Pierre-Yves David wrote:
>>> 
>>> 
>>> On 10/24/2014 08:07 PM, Gregory Szorc wrote:
>>>> # HG changeset patch
>>>> # User Gregory Szorc <gregory.szorc@gmail.com>
>>>> # Date 1414171261 25200
>>>> #      Fri Oct 24 10:21:01 2014 -0700
>>>> # Branch stable
>>>> # Node ID 8344ea64087d5eaab42709aaab6daa38dbde3db8
>>>> # Parent  eb763217152ab2b472416bcc57722451c317f282
>>>> tests: add test for failing bookmark push code
>>>> 
>>>> b901645a8784 regressed the behavior of pushing an unchanged bookmark to
>>>> a remote. Before that commit, pushing a unchanged bookmark would result
>>>> in "exporting bookmark @" being printed. After that commit, we now see
>>>> an incorrect message "bookmark %s does not exist on the local or remote
>>>> repository!"
>>> 
>>> I would argue that the "exporting bookmark X" is from if no change is
>>> actually made to the bookmark.
>> 
>> I considered this. But I wanted to write a backward-compatible patch to
>> minimize contention and increase chances of making 3.2.
> 
> I would argue that this "exporting bookmark @" when nothing is actually moved is a bug and worthy of fixing on stable.

I agree completely.

> 
> -- 
> Pierre-Yves David
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/tests/test-bookmarks-pushpull.t b/tests/test-bookmarks-pushpull.t
--- a/tests/test-bookmarks-pushpull.t
+++ b/tests/test-bookmarks-pushpull.t
@@ -437,4 +437,32 @@  pushing a new bookmark on a new head doe
   $ hg -R ../b id -r W
   cc978a373a53 tip W
 
   $ cd ..
+
+pushing an unchanged bookmark should result in no changes
+
+  $ hg init unchanged-a
+  $ hg init unchanged-b
+  $ cd unchanged-a
+  $ echo initial > foo
+  $ hg commit -A -m initial
+  adding foo
+  $ hg bookmark @
+  $ hg push -B @ ../unchanged-b
+  pushing to ../unchanged-b
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  exporting bookmark @
+
+BUGGY We should simply say "no changes found"
+  $ hg push -B @ ../unchanged-b
+  pushing to ../unchanged-b
+  searching for changes
+  bookmark @ does not exist on the local or remote repository!
+  no changes found
+  [2]
+
+  $ cd ..