unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH 0/2] test_expect_equal_file arguments check
@ 2012-02-02  0:04 Dmitry Kurochkin
  2012-02-02  0:04 ` [PATCH 1/2] test: add check for <expected> filename argument for test_expect_equal_file Dmitry Kurochkin
  2012-02-02  0:04 ` [PATCH 2/2] test: fix order and format of test_expect_equal_file arguments Dmitry Kurochkin
  0 siblings, 2 replies; 10+ messages in thread
From: Dmitry Kurochkin @ 2012-02-02  0:04 UTC (permalink / raw)
  To: notmuch

Hi all.

This is my second attempt to solve the test_expect_equal_file()
argument issue, based on Tomi's idea [1].  The first attempt and the
following discussion can be found here [2].

Regards,
  Dmitry

[1] id:"m28vknaq5l.fsf@guru.guru-group.fi"
[2] id:"1328080794-24670-1-git-send-email-dmitry.kurochkin@gmail.com"

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/2] test: add check for <expected> filename argument for test_expect_equal_file
  2012-02-02  0:04 [PATCH 0/2] test_expect_equal_file arguments check Dmitry Kurochkin
@ 2012-02-02  0:04 ` Dmitry Kurochkin
  2012-02-02 18:00   ` Jameson Graef Rollins
  2012-02-02  0:04 ` [PATCH 2/2] test: fix order and format of test_expect_equal_file arguments Dmitry Kurochkin
  1 sibling, 1 reply; 10+ messages in thread
From: Dmitry Kurochkin @ 2012-02-02  0:04 UTC (permalink / raw)
  To: notmuch

Test_expect_equal_file() function treats the first argument as "actual
output file" and the second argument as "expected output file".  When
the test fails, the files are copied for later inspection.  The first
files is copied to "$testname.output" and the second file to
"$testname.expected".  The argument order for test_expect_equal_file()
is often wrong which results in confusing diff output and incorrectly
named files.

The patch requires the <expected> filename argument for
test_expect_equal_file() function to contain 'EXPECT' or 'expect'
substring.  Otherwise test fails with appropriate error.  This allows
us to easily catch argument order errors during test development.

Most tests already comply with the required format.  But there are
some tests which are broken by this change.  Notably, the tests which
have wrong argument order for test_expect_equal_file() fail after the
change, which kind of proofs the idea.
---
 test/README      |    3 ++-
 test/test-lib.sh |    8 ++++++++
 2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/test/README b/test/README
index 43656a3..0105019 100644
--- a/test/README
+++ b/test/README
@@ -181,7 +181,8 @@ library for your script to use.
    Identical to test_exepect_equal, except that <output> and
    <expected> are files instead of strings.  This is a much more
    robust method to compare formatted textual information, since it
-   also notices whitespace and closing newline differences.
+   also notices whitespace and closing newline differences.  The
+   <expected> file name must contain 'expect' or 'EXPECT' substring.
 
  test_debug <script>
 
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 8158328..482f47b 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -490,6 +490,14 @@ test_expect_equal_file ()
 
 	output="$1"
 	expected="$2"
+	case "$expected" in
+	*expect*|*EXPECT*)
+		;;
+	*)
+		error "bug in the test script: test_expect_equal_file parameter '$expected' does not contain 'expect' substring"
+		;;
+	esac
+
 	if ! test_skip "$test_subtest_name"
 	then
 		if diff -q "$expected" "$output" >/dev/null ; then
-- 
1.7.9

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] test: fix order and format of test_expect_equal_file arguments
  2012-02-02  0:04 [PATCH 0/2] test_expect_equal_file arguments check Dmitry Kurochkin
  2012-02-02  0:04 ` [PATCH 1/2] test: add check for <expected> filename argument for test_expect_equal_file Dmitry Kurochkin
@ 2012-02-02  0:04 ` Dmitry Kurochkin
  1 sibling, 0 replies; 10+ messages in thread
From: Dmitry Kurochkin @ 2012-02-02  0:04 UTC (permalink / raw)
  To: notmuch

The patch fixes tests which were broken by the new
test_expect_equal_file() argument check.  Some tests have correct
filename format, but wrong argument order.  Others require files to be
renamed.
---
 test/crypto          |   10 +++++-----
 test/dump-restore    |   10 +++++-----
 test/hooks           |    8 ++++----
 test/multipart       |   20 ++++++++++----------
 test/search-limiting |   22 +++++++++++-----------
 test/symbol-hiding   |    4 ++--
 6 files changed, 37 insertions(+), 37 deletions(-)

diff --git a/test/crypto b/test/crypto
index 446a58b..9fdf710 100755
--- a/test/crypto
+++ b/test/crypto
@@ -140,14 +140,14 @@ test_expect_equal \
 mv "${GNUPGHOME}"{.bak,}
 
 # create a test encrypted message with attachment
-cat <<EOF >TESTATTACHMENT
+cat <<EOF >TESTATTACHMENT.expected
 This is a test file.
 EOF
 test_expect_success 'emacs delivery of encrypted message with attachment' \
 'emacs_deliver_message \
     "test encrypted message 001" \
     "This is a test encrypted message.\n" \
-    "(mml-attach-file \"TESTATTACHMENT\") (mml-secure-message-encrypt)"'
+    "(mml-attach-file \"TESTATTACHMENT.expected\") (mml-secure-message-encrypt)"'
 
 test_begin_subtest "decryption, --format=text"
 output=$(notmuch show --format=text --decrypt subject:"test encrypted message 001" \
@@ -170,7 +170,7 @@ Non-text part: application/pgp-encrypted
 \fpart{ ID: 4, Content-type: text/plain
 This is a test encrypted message.
 \fpart}
-\fattachment{ ID: 5, Filename: TESTATTACHMENT, Content-type: application/octet-stream
+\fattachment{ ID: 5, Filename: TESTATTACHMENT.expected, Content-type: application/octet-stream
 Non-text part: application/octet-stream
 \fattachment}
 \fpart}
@@ -210,7 +210,7 @@ expected='[[[{"id": "XXXXX",
  "content": "This is a test encrypted message.\n"},
  {"id": 5,
  "content-type": "application/octet-stream",
- "filename": "TESTATTACHMENT"}]}]}]},
+ "filename": "TESTATTACHMENT.expected"}]}]}]},
  []]]]'
 test_expect_equal \
     "$output" \
@@ -233,7 +233,7 @@ notmuch show \
     --part=5 \
     --decrypt \
     subject:"test encrypted message 001" >OUTPUT
-test_expect_equal_file OUTPUT TESTATTACHMENT
+test_expect_equal_file OUTPUT TESTATTACHMENT.expected
 
 test_begin_subtest "decryption failure with missing key"
 mv "${GNUPGHOME}"{,.bak}
diff --git a/test/dump-restore b/test/dump-restore
index 439e998..a6cfce2 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -55,7 +55,7 @@ test_expect_success 'Invalid restore invocation' \
 
 test_begin_subtest "dump outfile"
 notmuch dump dump-outfile.actual
-test_expect_equal_file dump.expected dump-outfile.actual
+test_expect_equal_file dump-outfile.actual dump.expected
 
 test_begin_subtest "dump outfile # deprecated"
 test_expect_equal "Warning: the output file argument of dump is deprecated."\
@@ -63,7 +63,7 @@ test_expect_equal "Warning: the output file argument of dump is deprecated."\
 
 test_begin_subtest "dump outfile --"
 notmuch dump dump-1-arg-dash.actual --
-test_expect_equal_file dump.expected dump-1-arg-dash.actual
+test_expect_equal_file dump-1-arg-dash.actual dump.expected
 
 # Note, we assume all messages from cworth have a message-id
 # containing cworth.org
@@ -72,14 +72,14 @@ grep 'cworth[.]org' dump.expected > dump-cworth.expected
 
 test_begin_subtest "dump -- from:cworth"
 notmuch dump -- from:cworth > dump-dash-cworth.actual
-test_expect_equal_file dump-cworth.expected dump-dash-cworth.actual
+test_expect_equal_file dump-dash-cworth.actual dump-cworth.expected
 
 test_begin_subtest "dump outfile from:cworth"
 notmuch dump dump-outfile-cworth.actual from:cworth
-test_expect_equal_file dump-cworth.expected dump-outfile-cworth.actual
+test_expect_equal_file dump-outfile-cworth.actual dump-cworth.expected
 
 test_begin_subtest "dump outfile -- from:cworth"
 notmuch dump dump-outfile-dash-inbox.actual -- from:cworth
-test_expect_equal_file dump-cworth.expected dump-outfile-dash-inbox.actual
+test_expect_equal_file dump-outfile-dash-inbox.actual dump-cworth.expected
 
 test_done
diff --git a/test/hooks b/test/hooks
index 77e8569..d084d1f 100755
--- a/test/hooks
+++ b/test/hooks
@@ -36,14 +36,14 @@ rm_hooks
 generate_message
 create_echo_hook "pre-new" expected output
 notmuch new > /dev/null
-test_expect_equal_file expected output
+test_expect_equal_file output expected
 
 test_begin_subtest "post-new is run"
 rm_hooks
 generate_message
 create_echo_hook "post-new" expected output
 notmuch new > /dev/null
-test_expect_equal_file expected output
+test_expect_equal_file output expected
 
 test_begin_subtest "pre-new is run before post-new"
 rm_hooks
@@ -51,7 +51,7 @@ generate_message
 create_echo_hook "pre-new" pre-new.expected pre-new.output
 create_echo_hook "post-new" post-new.expected post-new.output
 notmuch new > /dev/null
-test_expect_equal_file post-new.expected post-new.output
+test_expect_equal_file post-new.output post-new.expected
 
 test_begin_subtest "pre-new non-zero exit status (hook status)"
 rm_hooks
@@ -77,7 +77,7 @@ NOTMUCH_NEW 2>output.stderr >output
 cat output.stderr >> output
 echo "Added 1 new message to the database." > expected
 echo "Error: post-new hook failed with status 13" >> expected
-test_expect_equal_file expected output
+test_expect_equal_file output expected
 
 # depends on the previous subtest leaving broken hook behind
 test_expect_code 1 "post-new non-zero exit status (notmuch status)" "notmuch new"
diff --git a/test/multipart b/test/multipart
index 2dd73f5..c25cdf6 100755
--- a/test/multipart
+++ b/test/multipart
@@ -2,7 +2,7 @@
 test_description="output of multipart message"
 . ./test-lib.sh
 
-cat <<EOF > embedded_message
+cat <<EOF > embedded_message.expected
 From: Carl Worth <cworth@cworth.org>
 To: cworth@cworth.org
 Subject: html message
@@ -25,7 +25,7 @@ This is an embedded message, with a multipart/alternative part.
 --==-=-==--
 EOF
 
-cat <<EOF > ${MAIL_DIR}/multipart
+cat <<EOF > ${MAIL_DIR}/multipart.expected
 From: Carl Worth <cworth@cworth.org>
 To: cworth@cworth.org
 Subject: Multipart message
@@ -44,8 +44,8 @@ Content-Type: message/rfc822
 Content-Disposition: inline
 
 EOF
-cat embedded_message >> ${MAIL_DIR}/multipart
-cat <<EOF >> ${MAIL_DIR}/multipart
+cat embedded_message.expected >> ${MAIL_DIR}/multipart.expected
+cat <<EOF >> ${MAIL_DIR}/multipart.expected
 --=-=-=
 Content-Disposition: attachment; filename=attachment
 
@@ -108,7 +108,7 @@ notmuch new > /dev/null
 test_begin_subtest "--format=text --part=0, full message"
 notmuch show --format=text --part=0 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 cat <<EOF >EXPECTED
-\fmessage{ id:87liy5ap00.fsf@yoom.home.cworth.org depth:0 match:1 filename:${MAIL_DIR}/multipart
+\fmessage{ id:87liy5ap00.fsf@yoom.home.cworth.org depth:0 match:1 filename:${MAIL_DIR}/multipart.expected
 \fheader{
 Carl Worth <cworth@cworth.org> (2001-01-05) (attachment inbox signed unread)
 Subject: Multipart message
@@ -322,7 +322,7 @@ notmuch show --format=json --part=0 'id:87liy5ap00.fsf@yoom.home.cworth.org' | s
 echo >>OUTPUT # expect *no* newline at end of output
 cat <<EOF >EXPECTED
 
-{"id": "87liy5ap00.fsf@yoom.home.cworth.org", "match": true, "filename": "${MAIL_DIR}/multipart", "timestamp": 978709437, "date_relative": "2001-01-05", "tags": ["attachment","inbox","signed","unread"], "headers": {"Subject": "Multipart message", "From": "Carl Worth <cworth@cworth.org>", "To": "cworth@cworth.org", "Cc": "", "Bcc": "", "Date": "Fri, 05 Jan 2001 15:43:57 +0000"}, "body": [
+{"id": "87liy5ap00.fsf@yoom.home.cworth.org", "match": true, "filename": "${MAIL_DIR}/multipart.expected", "timestamp": 978709437, "date_relative": "2001-01-05", "tags": ["attachment","inbox","signed","unread"], "headers": {"Subject": "Multipart message", "From": "Carl Worth <cworth@cworth.org>", "To": "cworth@cworth.org", "Cc": "", "Bcc": "", "Date": "Fri, 05 Jan 2001 15:43:57 +0000"}, "body": [
 {"id": 1, "content-type": "multipart/signed", "content": [
 {"id": 2, "content-type": "multipart/mixed", "content": [
 {"id": 3, "content-type": "message/rfc822", "content": [{"headers": {"From": "Carl Worth <cworth@cworth.org>", "To": "cworth@cworth.org", "Subject": "html message", "Date": "Fri, 05 Jan 2001 15:42:57 +0000"}, "body": [
@@ -441,11 +441,11 @@ test_expect_success \
 
 test_begin_subtest "--format=raw"
 notmuch show --format=raw 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
-test_expect_equal_file OUTPUT "${MAIL_DIR}"/multipart
+test_expect_equal_file OUTPUT "${MAIL_DIR}"/multipart.expected
 
 test_begin_subtest "--format=raw --part=0, full message"
 notmuch show --format=raw --part=0 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
-test_expect_equal_file OUTPUT "${MAIL_DIR}"/multipart
+test_expect_equal_file OUTPUT "${MAIL_DIR}"/multipart.expected
 
 test_begin_subtest "--format=raw --part=1, message body"
 notmuch show --format=raw --part=1 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
@@ -494,7 +494,7 @@ test_begin_subtest "--format=raw --part=3, rfc822 part"
 test_subtest_known_broken
 
 notmuch show --format=raw --part=3 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
-test_expect_equal_file OUTPUT embedded_message
+test_expect_equal_file OUTPUT embedded_message.expected
 
 test_begin_subtest "--format=raw --part=4, rfc822's html part"
 notmuch show --format=raw --part=4 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
@@ -556,7 +556,7 @@ test_expect_success \
 test_begin_subtest "--format=mbox"
 notmuch show --format=mbox 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
 printf "From cworth@cworth.org Fri Jan  5 15:43:57 2001\n" >EXPECTED
-cat "${MAIL_DIR}"/multipart >>EXPECTED
+cat "${MAIL_DIR}"/multipart.expected >>EXPECTED
 # mbox output is expected to include a blank line
 echo >>EXPECTED
 test_expect_equal_file OUTPUT EXPECTED
diff --git a/test/search-limiting b/test/search-limiting
index 303762c..e93c76a 100755
--- a/test/search-limiting
+++ b/test/search-limiting
@@ -8,19 +8,19 @@ for outp in messages threads; do
     test_begin_subtest "${outp}: limit does the right thing"
     notmuch search --output=${outp} "*" | head -n 20 >expected
     notmuch search --output=${outp} --limit=20 "*" >output
-    test_expect_equal_file expected output
+    test_expect_equal_file output expected
 
     test_begin_subtest "${outp}: concatenation of limited searches"
     notmuch search --output=${outp} "*" | head -n 20 >expected
     notmuch search --output=${outp} --limit=10 "*" >output
     notmuch search --output=${outp} --limit=10 --offset=10 "*" >>output
-    test_expect_equal_file expected output
+    test_expect_equal_file output expected
 
     test_begin_subtest "${outp}: limit larger than result set"
     N=`notmuch count --output=${outp} "*"`
     notmuch search --output=${outp} "*" >expected
     notmuch search --output=${outp} --limit=$((1 + ${N})) "*" >output
-    test_expect_equal_file expected output
+    test_expect_equal_file output expected
 
     test_begin_subtest "${outp}: limit = 0"
     test_expect_equal "`notmuch search --output=${outp} --limit=0 "*"`" ""
@@ -29,43 +29,43 @@ for outp in messages threads; do
     # note: tail -n +N is 1-based
     notmuch search --output=${outp} "*" | tail -n +21 >expected
     notmuch search --output=${outp} --offset=20 "*" >output
-    test_expect_equal_file expected output
+    test_expect_equal_file output expected
 
     test_begin_subtest "${outp}: offset = 0"
     notmuch search --output=${outp} "*" >expected
     notmuch search --output=${outp} --offset=0 "*" >output
-    test_expect_equal_file expected output
+    test_expect_equal_file output expected
 
     test_begin_subtest "${outp}: negative offset"
     notmuch search --output=${outp} "*" | tail -n 20 >expected
     notmuch search --output=${outp} --offset=-20 "*" >output
-    test_expect_equal_file expected output
+    test_expect_equal_file output expected
 
     test_begin_subtest "${outp}: negative offset"
     notmuch search --output=${outp} "*" | tail -n 1 >expected
     notmuch search --output=${outp} --offset=-1 "*" >output
-    test_expect_equal_file expected output
+    test_expect_equal_file output expected
 
     test_begin_subtest "${outp}: negative offset combined with limit"
     notmuch search --output=${outp} "*" | tail -n 20 | head -n 10 >expected
     notmuch search --output=${outp} --offset=-20 --limit=10 "*" >output
-    test_expect_equal_file expected output
+    test_expect_equal_file output expected
 
     test_begin_subtest "${outp}: negative offset combined with equal limit"
     notmuch search --output=${outp} "*" | tail -n 20 >expected
     notmuch search --output=${outp} --offset=-20 --limit=20 "*" >output
-    test_expect_equal_file expected output
+    test_expect_equal_file output expected
 
     test_begin_subtest "${outp}: negative offset combined with large limit"
     notmuch search --output=${outp} "*" | tail -n 20 >expected
     notmuch search --output=${outp} --offset=-20 --limit=50 "*" >output
-    test_expect_equal_file expected output
+    test_expect_equal_file output expected
 
     test_begin_subtest "${outp}: negative offset larger then results"
     N=`notmuch count --output=${outp} "*"`
     notmuch search --output=${outp} "*" >expected
     notmuch search --output=${outp} --offset=-$((1 + ${N})) "*" >output
-    test_expect_equal_file expected output
+    test_expect_equal_file output expected
 done
 
 test_done
diff --git a/test/symbol-hiding b/test/symbol-hiding
index 636ec91..e6d896c 100755
--- a/test/symbol-hiding
+++ b/test/symbol-hiding
@@ -27,7 +27,7 @@ test_expect_equal "$result" "$output"
 
 test_begin_subtest 'comparing existing to exported symbols'
 objdump -t $TEST_DIRECTORY/../lib/*.o | awk '$4 == ".text" && $6 ~ "^notmuch" {print $6}' | sort | uniq > ACTUAL
-sed -n 's/[[:blank:]]*\(notmuch_[^;]*\);/\1/p' $TEST_DIRECTORY/../notmuch.sym | sort | uniq > EXPORTED
-test_expect_equal_file EXPORTED ACTUAL
+sed -n 's/[[:blank:]]*\(notmuch_[^;]*\);/\1/p' $TEST_DIRECTORY/../notmuch.sym | sort | uniq > EXPECTED
+test_expect_equal_file ACTUAL EXPECTED
 
 test_done
-- 
1.7.9

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] test: add check for <expected> filename argument for test_expect_equal_file
  2012-02-02  0:04 ` [PATCH 1/2] test: add check for <expected> filename argument for test_expect_equal_file Dmitry Kurochkin
@ 2012-02-02 18:00   ` Jameson Graef Rollins
  2012-02-03  0:18     ` Dmitry Kurochkin
  2012-02-03 14:04     ` Dmitry Kurochkin
  0 siblings, 2 replies; 10+ messages in thread
From: Jameson Graef Rollins @ 2012-02-02 18:00 UTC (permalink / raw)
  To: Dmitry Kurochkin, notmuch

[-- Attachment #1: Type: text/plain, Size: 424 bytes --]

Hey, Dmitry.  I'm so sorry I sent my last email on your original patch
before I saw this new series.  I do now like your original proposal
better, since it shows the diff based the names the caller provides,
which I now agree is probably the clearest and most robust solution.
The second patch in this series could still go through, though, no
matter what version of the change to test_expect_equal_file we go with.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] test: add check for <expected> filename argument for test_expect_equal_file
  2012-02-02 18:00   ` Jameson Graef Rollins
@ 2012-02-03  0:18     ` Dmitry Kurochkin
  2012-02-03 14:04     ` Dmitry Kurochkin
  1 sibling, 0 replies; 10+ messages in thread
From: Dmitry Kurochkin @ 2012-02-03  0:18 UTC (permalink / raw)
  To: Jameson Graef Rollins, notmuch

Hi Jameson.

On Thu, 02 Feb 2012 10:00:59 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Hey, Dmitry.  I'm so sorry I sent my last email on your original patch
> before I saw this new series.

No problems.

>  I do now like your original proposal
> better, since it shows the diff based the names the caller provides,
> which I now agree is probably the clearest and most robust solution.
> The second patch in this series could still go through, though, no
> matter what version of the change to test_expect_equal_file we go with.
> 

Ok.  I am going to wait for some time (e.g. 2 days) for more opinions.
Though, I doubt I would get any :) (And the problem is really minor, so
it probably does not deserve much attention.)

Currently, it seems that the original approach [1] wins.

Regards,
  Dmitry

[1] id:"1328080794-24670-1-git-send-email-dmitry.kurochkin@gmail.com"

> jamie.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] test: add check for <expected> filename argument for test_expect_equal_file
  2012-02-02 18:00   ` Jameson Graef Rollins
  2012-02-03  0:18     ` Dmitry Kurochkin
@ 2012-02-03 14:04     ` Dmitry Kurochkin
  2012-02-03 21:24       ` Jameson Graef Rollins
                         ` (2 more replies)
  1 sibling, 3 replies; 10+ messages in thread
From: Dmitry Kurochkin @ 2012-02-03 14:04 UTC (permalink / raw)
  To: Jameson Graef Rollins, notmuch

On Thu, 02 Feb 2012 10:00:59 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Hey, Dmitry.  I'm so sorry I sent my last email on your original patch
> before I saw this new series.  I do now like your original proposal
> better, since it shows the diff based the names the caller provides,
> which I now agree is probably the clearest and most robust solution.
> The second patch in this series could still go through, though, no
> matter what version of the change to test_expect_equal_file we go with.
> 

Actually, we can do both: check file name for consistent diff order
(from expected to actual) and use file names that the caller provides.

What do you think?

Regards,
  Dmitry

> jamie.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] test: add check for <expected> filename argument for test_expect_equal_file
  2012-02-03 14:04     ` Dmitry Kurochkin
@ 2012-02-03 21:24       ` Jameson Graef Rollins
  2012-02-04  8:28       ` Tomi Ollila
  2012-10-12 17:42       ` Ethan Glasser-Camp
  2 siblings, 0 replies; 10+ messages in thread
From: Jameson Graef Rollins @ 2012-02-03 21:24 UTC (permalink / raw)
  To: Dmitry Kurochkin, notmuch

[-- Attachment #1: Type: text/plain, Size: 329 bytes --]

On Fri, 03 Feb 2012 18:04:05 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Actually, we can do both: check file name for consistent diff order
> (from expected to actual) and use file names that the caller provides.
> 
> What do you think?

I'm not sure it's worth it, but I suppose that's fine.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] test: add check for <expected> filename argument for test_expect_equal_file
  2012-02-03 14:04     ` Dmitry Kurochkin
  2012-02-03 21:24       ` Jameson Graef Rollins
@ 2012-02-04  8:28       ` Tomi Ollila
  2012-10-12 17:42       ` Ethan Glasser-Camp
  2 siblings, 0 replies; 10+ messages in thread
From: Tomi Ollila @ 2012-02-04  8:28 UTC (permalink / raw)
  To: Dmitry Kurochkin, notmuch

On Fri, 03 Feb 2012 18:04:05 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> On Thu, 02 Feb 2012 10:00:59 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> > Hey, Dmitry.  I'm so sorry I sent my last email on your original patch
> > before I saw this new series.  I do now like your original proposal
> > better, since it shows the diff based the names the caller provides,
> > which I now agree is probably the clearest and most robust solution.
> > The second patch in this series could still go through, though, no
> > matter what version of the change to test_expect_equal_file we go with.
> > 
> 
> Actually, we can do both: check file name for consistent diff order
> (from expected to actual) and use file names that the caller provides.
> 
> What do you think?

+1

> Regards,
>   Dmitry

Tomi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] test: add check for <expected> filename argument for test_expect_equal_file
  2012-02-03 14:04     ` Dmitry Kurochkin
  2012-02-03 21:24       ` Jameson Graef Rollins
  2012-02-04  8:28       ` Tomi Ollila
@ 2012-10-12 17:42       ` Ethan Glasser-Camp
  2012-10-18 10:05         ` Tomi Ollila
  2 siblings, 1 reply; 10+ messages in thread
From: Ethan Glasser-Camp @ 2012-10-12 17:42 UTC (permalink / raw)
  To: Dmitry Kurochkin, Jameson Graef Rollins, notmuch

Dmitry Kurochkin <dmitry.kurochkin@gmail.com> writes:

> Actually, we can do both: check file name for consistent diff order
> (from expected to actual) and use file names that the caller provides.

Hi! Reviewing the patch queue a little bit here. It seems like this
patch ended up getting dropped because the other approach (using the
filenames that the caller provided) won out and was even pushed. I'm
therefore marking this series notmuch::obsolete (and also
notmuch::stale since it doesn't apply cleanly).

Ethan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] test: add check for <expected> filename argument for test_expect_equal_file
  2012-10-12 17:42       ` Ethan Glasser-Camp
@ 2012-10-18 10:05         ` Tomi Ollila
  0 siblings, 0 replies; 10+ messages in thread
From: Tomi Ollila @ 2012-10-18 10:05 UTC (permalink / raw)
  To: Ethan Glasser-Camp, notmuch

On Fri, Oct 12 2012, Ethan Glasser-Camp wrote:

> Dmitry Kurochkin <dmitry.kurochkin@gmail.com> writes:
>
>> Actually, we can do both: check file name for consistent diff order
>> (from expected to actual) and use file names that the caller provides.
>
> Hi! Reviewing the patch queue a little bit here. It seems like this
> patch ended up getting dropped because the other approach (using the
> filenames that the caller provided) won out and was even pushed. I'm
> therefore marking this series notmuch::obsolete (and also
> notmuch::stale since it doesn't apply cleanly).

I think stale is unnecessary and inconvenient when patch is marked
obsolete

1) it is easier to look for stale patches by just searching stale tag
2) most obsolete patch goes stale at later time

If no-one resists I'll execute

notmuch tag -notmuch::stale tag:notmuch::obsolete

in near future (I take silence as an approval :D).

Currently:

notmuch search --output=messages tag:notmuch::stale and tag:notmuch::obsolete

id:1334077496-9172-3-git-send-email-markwalters1009@gmail.com
id:1334077496-9172-2-git-send-email-markwalters1009@gmail.com
id:1331402091-15663-1-git-send-email-tom.prince@ualberta.net
id:1328141050-30356-2-git-send-email-dmitry.kurochkin@gmail.com
id:1327985666-29191-3-git-send-email-dmitry.kurochkin@gmail.com
id:1327961195-4204-2-git-send-email-dmitry.kurochkin@gmail.com
id:1327926286-16680-2-git-send-email-dmitry.kurochkin@gmail.com
id:1325986015-22510-5-git-send-email-jrollins@finestructure.net


Tomi

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-10-18 10:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-02  0:04 [PATCH 0/2] test_expect_equal_file arguments check Dmitry Kurochkin
2012-02-02  0:04 ` [PATCH 1/2] test: add check for <expected> filename argument for test_expect_equal_file Dmitry Kurochkin
2012-02-02 18:00   ` Jameson Graef Rollins
2012-02-03  0:18     ` Dmitry Kurochkin
2012-02-03 14:04     ` Dmitry Kurochkin
2012-02-03 21:24       ` Jameson Graef Rollins
2012-02-04  8:28       ` Tomi Ollila
2012-10-12 17:42       ` Ethan Glasser-Camp
2012-10-18 10:05         ` Tomi Ollila
2012-02-02  0:04 ` [PATCH 2/2] test: fix order and format of test_expect_equal_file arguments Dmitry Kurochkin

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).