Patchwork [2,of,2] tests: simplify and clarify test-obsolete-bundle-strip.t a little

login
register
mail settings
Submitter via Mercurial-devel
Date June 3, 2017, 7:35 a.m.
Message ID <ff06148cbf34428d71bd.1496475319@martinvonz.svl.corp.google.com>
Download mbox | patch
Permalink /patch/21147/
State Superseded
Headers show

Comments

via Mercurial-devel - June 3, 2017, 7:35 a.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1496469903 25200
#      Fri Jun 02 23:05:03 2017 -0700
# Node ID ff06148cbf34428d71bd7ea9d2e31fa806686417
# Parent  f782e04fb7d18868cf7da00a41aaa8f15c8e8802
tests: simplify and clarify test-obsolete-bundle-strip.t a little
Yuya Nishihara - June 3, 2017, 3:21 p.m.
On Sat, 03 Jun 2017 00:35:19 -0700, Martin von Zweigbergk via Mercurial-devel wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1496469903 25200
> #      Fri Jun 02 23:05:03 2017 -0700
> # Node ID ff06148cbf34428d71bd7ea9d2e31fa806686417
> # Parent  f782e04fb7d18868cf7da00a41aaa8f15c8e8802
> tests: simplify and clarify test-obsolete-bundle-strip.t a little
> 
> diff --git a/tests/test-obsolete-bundle-strip.t b/tests/test-obsolete-bundle-strip.t
> --- a/tests/test-obsolete-bundle-strip.t
> +++ b/tests/test-obsolete-bundle-strip.t
> @@ -76,8 +76,8 @@
>    >     echo '### Exclusive markers ###'
>    >     cat "${exclufile}"
>    >     # if the matched revs do not have children, we also check the result of strip
> -  >     orphan=`hg log --hidden -T '.\n' --rev "(not (${revs})) and (${revs}::)" | wc -l | sed -e 's/ //g'`
> -  >     if [ $orphan -eq 0 ];
> +  >     children=`hg log --hidden -T . --rev "(${revs}::) - (${revs})" | wc -c`
> +  >     if [ $children -eq 0 ];

Perhaps this can be simplified further. drop "wc -c" and test [ -z "$children" ].
via Mercurial-devel - June 3, 2017, 4:01 p.m.
On Jun 3, 2017 8:22 AM, "Yuya Nishihara" <yuya@tcha.org> wrote:

On Sat, 03 Jun 2017 00:35:19 -0700, Martin von Zweigbergk via
Mercurial-devel wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1496469903 25200
> #      Fri Jun 02 23:05:03 2017 -0700
> # Node ID ff06148cbf34428d71bd7ea9d2e31fa806686417
> # Parent  f782e04fb7d18868cf7da00a41aaa8f15c8e8802
> tests: simplify and clarify test-obsolete-bundle-strip.t a little
>
> diff --git a/tests/test-obsolete-bundle-strip.t
b/tests/test-obsolete-bundle-strip.t
> --- a/tests/test-obsolete-bundle-strip.t
> +++ b/tests/test-obsolete-bundle-strip.t
> @@ -76,8 +76,8 @@
>    >     echo '### Exclusive markers ###'
>    >     cat "${exclufile}"
>    >     # if the matched revs do not have children, we also check the
result of strip
> -  >     orphan=`hg log --hidden -T '.\n' --rev "(not (${revs})) and
(${revs}::)" | wc -l | sed -e 's/ //g'`
> -  >     if [ $orphan -eq 0 ];
> +  >     children=`hg log --hidden -T . --rev "(${revs}::) - (${revs})" |
wc -c`
> +  >     if [ $children -eq 0 ];

Perhaps this can be simplified further. drop "wc -c" and test [ -z
"$children" ].


Yes, I considered that but thought that maybe Pierre-Yves wanted the faster
'.' template. But that's a pretty silly optimization to do in a test, so
I'll do as you suggested in the next version.
Pierre-Yves David - June 3, 2017, 4:08 p.m.
On 06/03/2017 06:01 PM, Martin von Zweigbergk via Mercurial-devel wrote:
>
>
> On Jun 3, 2017 8:22 AM, "Yuya Nishihara" <yuya@tcha.org
> <mailto:yuya@tcha.org>> wrote:
>
>     On Sat, 03 Jun 2017 00:35:19 -0700, Martin von Zweigbergk via
>     Mercurial-devel wrote:
>     > # HG changeset patch
>     > # User Martin von Zweigbergk <martinvonz@google.com
>     <mailto:martinvonz@google.com>>
>     > # Date 1496469903 25200
>     > #      Fri Jun 02 23:05:03 2017 -0700
>     > # Node ID ff06148cbf34428d71bd7ea9d2e31fa806686417
>     > # Parent  f782e04fb7d18868cf7da00a41aaa8f15c8e8802
>     > tests: simplify and clarify test-obsolete-bundle-strip.t a little
>     >
>     > diff --git a/tests/test-obsolete-bundle-strip.t
>     b/tests/test-obsolete-bundle-strip.t
>     > --- a/tests/test-obsolete-bundle-strip.t
>     > +++ b/tests/test-obsolete-bundle-strip.t
>     > @@ -76,8 +76,8 @@
>     >    >     echo '### Exclusive markers ###'
>     >    >     cat "${exclufile}"
>     >    >     # if the matched revs do not have children, we also check
>     the result of strip
>     > -  >     orphan=`hg log --hidden -T '.\n' --rev "(not (${revs}))
>     and (${revs}::)" | wc -l | sed -e 's/ //g'`
>     > -  >     if [ $orphan -eq 0 ];
>     > +  >     children=`hg log --hidden -T . --rev "(${revs}::) -
>     (${revs})" | wc -c`
>     > +  >     if [ $children -eq 0 ];
>
>     Perhaps this can be simplified further. drop "wc -c" and test [ -z
>     "$children" ].
>
>
> Yes, I considered that but thought that maybe Pierre-Yves wanted the
> faster '.' template. But that's a pretty silly optimization to do in a
> test, so I'll do as you suggested in the next version.

Yuya version seems fine. No real optimization concerns here, I just took 
the "counting number of changeset matching a revset" pattern of the 
shelve of my mental library without thinking too hard. Thanks for the 
clean up. The test update looks good.

(+1 to yuya pointing out the other ${rev} needs () too)

Patch

diff --git a/tests/test-obsolete-bundle-strip.t b/tests/test-obsolete-bundle-strip.t
--- a/tests/test-obsolete-bundle-strip.t
+++ b/tests/test-obsolete-bundle-strip.t
@@ -76,8 +76,8 @@ 
   >     echo '### Exclusive markers ###'
   >     cat "${exclufile}"
   >     # if the matched revs do not have children, we also check the result of strip
-  >     orphan=`hg log --hidden -T '.\n' --rev "(not (${revs})) and (${revs}::)" | wc -l | sed -e 's/ //g'`
-  >     if [ $orphan -eq 0 ];
+  >     children=`hg log --hidden -T . --rev "(${revs}::) - (${revs})" | wc -c`
+  >     if [ $children -eq 0 ];
   >     then
   >         printf "# stripping: "
   >         prestripfile="${prefix}-pre-strip.txt"