Re: [PATCH v3 4/4] ref-filter: add support for %(contents:size)

[Christian Couder <christian.couder@gmail.com>]

On Thu, Jul 9, 2020 at 2:14 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Of course, we _could_ update the test_atom to do something magic
> > only when the 'contents' atom is being asked.  We notice that $2 is
> > 'contents', do the usual test_expect_success for 'contents', and
> > then measure the byte length of $3 ourselves and test
> > 'contents:size'.  That way, all the above test updates would become
> > unnecessary (and the last two hunks of this patch can also go).
> >
> > That approach may even allow you to hide the details of sanitize-pgp
> > in the updated test_atom so that the actual tests may not have to get
> > updated even for signed tags.
>
> After seeing the "wc -c" portability issues, I am now even more
> inclined to say that the above is the right direction.  The
> portability worries can and should be encapsulated in a single
> test_atom helper function, just as it can be used to hide the
> differences between signed tags, annotated tags and commits.

Yeah, I have been working on that and will send a new patch series soon.
The current test_atom() change looks like this:

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 371e45e5ad..e514d98574 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -52,6 +52,26 @@ test_atom() {
                sanitize_pgp <actual >actual.clean &&
                test_cmp expected actual.clean
        "
+       # Automatically test "contents:size" atom after testing "contents"
+       if test "$2" = "contents"
+       then
+               case "$1" in
+               refs/tags/signed-*)
+                       # We cannot use $3 as it expects sanitize_pgp to run
+                       git cat-file tag $ref | tail -n +6 | \
+                               wc -c | sed -e 's/^ *//' >expected ;;
+               refs/mytrees/*)
+                       echo >expected ;;
+               refs/myblobs/*)
+                       echo >expected ;;
+               *)
+                       printf '%s' "$3" | wc -c | sed -e 's/^ *//' >expected ;;
+               esac
+               test_expect_${4:-success} $PREREQ "basic atom: $1 $2:size" "
+                       git for-each-ref --format='%($2:size)' $ref >actual &&
+                       test_cmp expected actual
+               "
+       fi
 }

I am wondering if it's worth adding a preparatory patch to introduce
an helper function like the following in test-lib-functions.sh:

+# test_byte_count outputs the number of bytes in files or stdin
+#
+# It is like wc -c but without portability issues, as on macOS and
+# possibly other platforms leading whitespaces are emitted before the
+# number.
+
+test_byte_count () {
+       wc -c "$@" | sed -e 's/^ *//'
+}

Not sure about the name of this helper function as it works
differently than test_line_count().