From: "Torsten Bögershausen" <tboegi@web.de>
To: Lars Schneider <larsxschneider@gmail.com>
Cc: "Junio C Hamano" <gitster@pobox.com>, git <git@vger.kernel.org>,
"Jakub Narębski" <jnareb@gmail.com>,
peff@peff.net
Subject: Re: [PATCH v10 13/14] convert: add filter.<driver>.process option
Date: Tue, 11 Oct 2016 10:09:46 +0000 [thread overview]
Message-ID: <20161011100946.GA13745@tb-raspi> (raw)
In-Reply-To: <02E73D87-B036-40CA-AF54-F93415A028BC@gmail.com>
On Tue, Oct 11, 2016 at 10:11:22AM +0200, Lars Schneider wrote:
>
> > On 10 Oct 2016, at 21:58, Junio C Hamano <gitster@pobox.com> wrote:
> >
> > larsxschneider@gmail.com writes:
> >
> > [...]
> >> +# Count unique lines except clean invocations in two files and compare
> >> +# them. Clean invocations are not counted because their number can vary.
> >> +# c.f. http://public-inbox.org/git/xmqqshv18i8i.fsf@gitster.mtv.corp.google.com/
> >> +test_cmp_count_except_clean () {
> >> + for FILE in $@
> >> + do
> >> + sort $FILE | uniq -c | sed "s/^[ ]*//" |
> >> + sed "s/^\([0-9]\) IN: clean/x IN: clean/" >$FILE.tmp
> >> + cat $FILE.tmp >$FILE
> >> + done &&
> >> + test_cmp $@
> >> +}
> >
> > Why do you even _care_ about the number of invocations? While I
> > told you why "clean" could be called multiple times under racy Git
> > as an example, that was not meant to be an exhaustive example. I
> > wouldn't be surprised if we needed to run smudge twice, for example,
> > in some weirdly racy cases in the future.
> >
> > Can we just have the correctness (i.e. "we expect that the working
> > tree file gets this as the result of checking it out, and we made
> > sure that is the case") test without getting into such an
> > implementation detail?
>
> My goal is to check that clean/smudge is invoked at least once. I could
> just run `uniq` to achieve that but then all other filter commands could
> happen multiple times and the test would not detect that.
>
> I also prefer to check the filter commands to ensure the filter is
> working as expected (e.g. no multiple start ups etc) in addition to
> checking the working tree.
>
> Would the patch below work for you? If yes, then please squash it into
> "convert: add filter.<driver>.process option".
>
> Thank you,
> Lars
>
>
>
> diff --git a/t/t0021-conversion.sh b/t/t0021-conversion.sh
> index 9f892c0..714f706 100755
> --- a/t/t0021-conversion.sh
> +++ b/t/t0021-conversion.sh
> @@ -31,38 +31,33 @@ filter_git () {
> rm -f git-stderr.log
> }
>
> -# Count unique lines in two files and compare them.
> -test_cmp_count () {
> - for FILE in $@
> - do
> - sort $FILE | uniq -c | sed "s/^[ ]*//" >$FILE.tmp
> - cat $FILE.tmp >$FILE
> - done &&
> - test_cmp $@
> -}
> -
> -# Count unique lines except clean invocations in two files and compare
> -# them. Clean invocations are not counted because their number can vary.
> +# Compare two files and ensure that `clean` and `smudge` respectively are
> +# called at least once if specified in the `expect` file. The actual
> +# invocation count is not relevant because their number can vary.
> # c.f. http://public-inbox.org/git/xmqqshv18i8i.fsf@gitster.mtv.corp.google.com/
> -test_cmp_count_except_clean () {
> - for FILE in $@
> +test_cmp_count () {
> + expect=$1 actual=$2
That could be
expect="$1"
actual="$2"
> + for FILE in "$expect" "$actual"
> do
> + sort "$FILE" | uniq -c | sed "s/^[ ]*//" |
> + sed "s/^\([0-9]\) IN: clean/x IN: clean/" |
> + sed "s/^\([0-9]\) IN: smudge/x IN: smudge/" >"$FILE.tmp" &&
> + cat "$FILE.tmp" >"$FILE"
How about
cp "$FILE.tmp" "$FILE"
next prev parent reply other threads:[~2016-10-11 10:18 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-08 11:25 [PATCH v10 00/14] Git filter protocol larsxschneider
2016-10-08 11:25 ` [PATCH v10 01/14] convert: quote filter names in error messages larsxschneider
2016-10-08 11:25 ` [PATCH v10 02/14] convert: modernize tests larsxschneider
2016-10-08 11:25 ` [PATCH v10 03/14] run-command: move check_pipe() from write_or_die to run_command larsxschneider
2016-10-08 11:25 ` [PATCH v10 04/14] run-command: add clean_on_exit_handler larsxschneider
2016-10-11 12:12 ` Johannes Schindelin
2016-10-15 15:02 ` Lars Schneider
2016-10-16 8:03 ` Johannes Schindelin
2016-10-16 21:57 ` Lars Schneider
2016-10-08 11:25 ` [PATCH v10 05/14] pkt-line: rename packet_write() to packet_write_fmt() larsxschneider
2016-10-08 11:25 ` [PATCH v10 06/14] pkt-line: extract set_packet_header() larsxschneider
2016-10-08 11:25 ` [PATCH v10 07/14] pkt-line: add packet_write_fmt_gently() larsxschneider
2016-10-08 11:25 ` [PATCH v10 08/14] pkt-line: add packet_flush_gently() larsxschneider
2016-10-08 11:25 ` [PATCH v10 09/14] pkt-line: add packet_write_gently() larsxschneider
2016-10-08 11:25 ` [PATCH v10 10/14] pkt-line: add functions to read/write flush terminated packet streams larsxschneider
2016-10-08 11:25 ` [PATCH v10 11/14] convert: make apply_filter() adhere to standard Git error handling larsxschneider
2016-10-08 11:25 ` [PATCH v10 12/14] convert: prepare filter.<driver>.process option larsxschneider
2016-10-08 11:25 ` [PATCH v10 13/14] convert: add " larsxschneider
2016-10-08 23:06 ` Jakub Narębski
2016-10-09 5:32 ` Torsten Bögershausen
2016-10-11 15:29 ` Lars Schneider
2016-10-11 22:26 ` Lars Schneider
2016-10-12 10:54 ` Jakub Narębski
2016-10-15 14:45 ` Lars Schneider
2016-10-15 17:41 ` Jeff King
2016-10-15 19:42 ` Jakub Narębski
2016-10-10 19:58 ` Junio C Hamano
2016-10-11 8:11 ` Lars Schneider
2016-10-11 10:09 ` Torsten Bögershausen [this message]
2016-10-16 23:13 ` Lars Schneider
2016-10-17 17:05 ` Junio C Hamano
2016-10-08 11:25 ` [PATCH v10 14/14] contrib/long-running-filter: add long running filter example larsxschneider
2016-10-09 5:42 ` Torsten Bögershausen
2016-10-15 14:47 ` Lars Schneider
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161011100946.GA13745@tb-raspi \
--to=tboegi@web.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jnareb@gmail.com \
--cc=larsxschneider@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.