[PATCH 0/9] test: ruby: several cleanups and simplifications

classic Classic list List threaded Threaded
26 messages Options
12
Felipe Contreras Felipe Contreras
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/9] test: ruby: several cleanups and simplifications

I found a lot of areas of improvement in the Ruby tests, so I decided to clean them up.

With these changes the tests are now much simpler and follow more closely the typical Ruby idioms.

Felipe Contreras (9):
  test: move test_ruby() inside the only client
  test: ruby: refactor test_ruby()
  test: ruby: simplify MAIL_DIR check
  test: ruby: simplify MAIL_DIR initialization
  test: ruby: simplify test_ruby()
  test: ruby: use much more standard puts
  test: ruby: use much more standard Ruby idioms
  test: ruby: don't use instance variables
  test: ruby: simplify output comparison

 test/T395-ruby.sh | 94 +++++++++++++++--------------------------------
 test/test-lib.sh  |  4 --
 2 files changed, 30 insertions(+), 68 deletions(-)

--
2.31.0
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
Felipe Contreras Felipe Contreras
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/9] test: move test_ruby() inside the only client

Not much point in polluting the main library, and also will be useful to
modify it in tandem with the tests.

Signed-off-by: Felipe Contreras <[hidden email]>
---
 test/T395-ruby.sh | 4 ++++
 test/test-lib.sh  | 4 ----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/T395-ruby.sh b/test/T395-ruby.sh
index a0b76eb8..fec1f5ef 100755
--- a/test/T395-ruby.sh
+++ b/test/T395-ruby.sh
@@ -8,6 +8,10 @@ fi
 
 add_email_corpus
 
+test_ruby() {
+    MAIL_DIR=$MAIL_DIR $NOTMUCH_RUBY -I "$NOTMUCH_BUILDDIR/bindings/ruby"> OUTPUT
+}
+
 test_begin_subtest "compare thread ids"
 test_ruby <<"EOF"
 require 'notmuch'
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 4c9f2a21..ec0ba7f7 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -1109,10 +1109,6 @@ test_python() {
  $NOTMUCH_PYTHON -B - > OUTPUT
 }
 
-test_ruby() {
-    MAIL_DIR=$MAIL_DIR $NOTMUCH_RUBY -I "$NOTMUCH_BUILDDIR/bindings/ruby"> OUTPUT
-}
-
 test_C () {
     local exec_file test_file
     exec_file="test${test_count}"
--
2.31.0
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
Felipe Contreras Felipe Contreras
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/9] test: ruby: refactor test_ruby()

In reply to this post by Felipe Contreras
There's no point in repeating the same initialization in all the tests.

Signed-off-by: Felipe Contreras <[hidden email]>
---
 test/T395-ruby.sh | 48 +++++++++++------------------------------------
 1 file changed, 11 insertions(+), 37 deletions(-)

diff --git a/test/T395-ruby.sh b/test/T395-ruby.sh
index fec1f5ef..1d27e191 100755
--- a/test/T395-ruby.sh
+++ b/test/T395-ruby.sh
@@ -9,17 +9,21 @@ fi
 add_email_corpus
 
 test_ruby() {
-    MAIL_DIR=$MAIL_DIR $NOTMUCH_RUBY -I "$NOTMUCH_BUILDDIR/bindings/ruby"> OUTPUT
+    (
+ cat <<-\EOF
+ require 'notmuch'
+ $maildir = ENV['MAIL_DIR']
+ if not $maildir then
+  abort('environment variable MAIL_DIR must be set')
+ end
+ @db = Notmuch::Database.new($maildir)
+ EOF
+ cat
+    ) | MAIL_DIR=$MAIL_DIR $NOTMUCH_RUBY -I "$NOTMUCH_BUILDDIR/bindings/ruby"> OUTPUT
 }
 
 test_begin_subtest "compare thread ids"
 test_ruby <<"EOF"
-require 'notmuch'
-$maildir = ENV['MAIL_DIR']
-if not $maildir then
-  abort('environment variable MAIL_DIR must be set')
-end
-@db = Notmuch::Database.new($maildir)
 @q = @db.query('tag:inbox')
 @q.sort = Notmuch::SORT_OLDEST_FIRST
 for t in @q.search_threads do
@@ -31,12 +35,6 @@ test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "compare message ids"
 test_ruby <<"EOF"
-require 'notmuch'
-$maildir = ENV['MAIL_DIR']
-if not $maildir then
-  abort('environment variable MAIL_DIR must be set')
-end
-@db = Notmuch::Database.new($maildir)
 @q = @db.query('tag:inbox')
 @q.sort = Notmuch::SORT_OLDEST_FIRST
 for m in @q.search_messages do
@@ -48,12 +46,6 @@ test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "get non-existent file"
 test_ruby <<"EOF"
-require 'notmuch'
-$maildir = ENV['MAIL_DIR']
-if not $maildir then
-  abort('environment variable MAIL_DIR must be set')
-end
-@db = Notmuch::Database.new($maildir)
 result = @db.find_message_by_filename('i-dont-exist')
 print (result == nil)
 EOF
@@ -61,12 +53,6 @@ test_expect_equal "$(cat OUTPUT)" "true"
 
 test_begin_subtest "count messages"
 test_ruby <<"EOF"
-require 'notmuch'
-$maildir = ENV['MAIL_DIR']
-if not $maildir then
-  abort('environment variable MAIL_DIR must be set')
-end
-@db = Notmuch::Database.new($maildir)
 @q = @db.query('tag:inbox')
 print @q.count_messages(),"\n"
 EOF
@@ -75,12 +61,6 @@ test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "count threads"
 test_ruby <<"EOF"
-require 'notmuch'
-$maildir = ENV['MAIL_DIR']
-if not $maildir then
-  abort('environment variable MAIL_DIR must be set')
-end
-@db = Notmuch::Database.new($maildir)
 @q = @db.query('tag:inbox')
 print @q.count_threads(),"\n"
 EOF
@@ -89,12 +69,6 @@ test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "get all tags"
 test_ruby <<"EOF"
-require 'notmuch'
-$maildir = ENV['MAIL_DIR']
-if not $maildir then
-  abort('environment variable MAIL_DIR must be set')
-end
-@db = Notmuch::Database.new($maildir)
 @t = @db.all_tags()
 for tag in @t do
    print tag,"\n"
--
2.31.0
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
Felipe Contreras Felipe Contreras
Reply | Threaded
Open this post in threaded view
|

[PATCH 3/9] test: ruby: simplify MAIL_DIR check

In reply to this post by Felipe Contreras
Signed-off-by: Felipe Contreras <[hidden email]>
---
 test/T395-ruby.sh | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/test/T395-ruby.sh b/test/T395-ruby.sh
index 1d27e191..94fab106 100755
--- a/test/T395-ruby.sh
+++ b/test/T395-ruby.sh
@@ -12,10 +12,7 @@ test_ruby() {
     (
  cat <<-\EOF
  require 'notmuch'
- $maildir = ENV['MAIL_DIR']
- if not $maildir then
-  abort('environment variable MAIL_DIR must be set')
- end
+ $maildir = ENV['MAIL_DIR'] || abort('MAIL_DIR not set')
  @db = Notmuch::Database.new($maildir)
  EOF
  cat
--
2.31.0
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
Felipe Contreras Felipe Contreras
Reply | Threaded
Open this post in threaded view
|

[PATCH 4/9] test: ruby: simplify MAIL_DIR initialization

In reply to this post by Felipe Contreras
There's no need to complicate the script passing the MAIL_DIR
environment variable.

Signed-off-by: Felipe Contreras <[hidden email]>
---
 test/T395-ruby.sh | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/test/T395-ruby.sh b/test/T395-ruby.sh
index 94fab106..67d6e205 100755
--- a/test/T395-ruby.sh
+++ b/test/T395-ruby.sh
@@ -10,13 +10,12 @@ add_email_corpus
 
 test_ruby() {
     (
- cat <<-\EOF
+ cat <<-EOF
  require 'notmuch'
- $maildir = ENV['MAIL_DIR'] || abort('MAIL_DIR not set')
- @db = Notmuch::Database.new($maildir)
+ @db = Notmuch::Database.new('$MAIL_DIR')
  EOF
  cat
-    ) | MAIL_DIR=$MAIL_DIR $NOTMUCH_RUBY -I "$NOTMUCH_BUILDDIR/bindings/ruby"> OUTPUT
+    ) | $NOTMUCH_RUBY -I "$NOTMUCH_BUILDDIR/bindings/ruby"> OUTPUT
 }
 
 test_begin_subtest "compare thread ids"
--
2.31.0
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
Felipe Contreras Felipe Contreras
Reply | Threaded
Open this post in threaded view
|

[PATCH 5/9] test: ruby: simplify test_ruby()

In reply to this post by Felipe Contreras
We always do test_expect_equal_file, so do it in test_ruby() directly.

The only subtest where we don't (get non-existent file) can be easily
modified.

Signed-off-by: Felipe Contreras <[hidden email]>
---
 test/T395-ruby.sh | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/test/T395-ruby.sh b/test/T395-ruby.sh
index 67d6e205..55bf4c2b 100755
--- a/test/T395-ruby.sh
+++ b/test/T395-ruby.sh
@@ -16,9 +16,11 @@ test_ruby() {
  EOF
  cat
     ) | $NOTMUCH_RUBY -I "$NOTMUCH_BUILDDIR/bindings/ruby"> OUTPUT
+    test_expect_equal_file EXPECTED OUTPUT
 }
 
 test_begin_subtest "compare thread ids"
+notmuch search --sort=oldest-first --output=threads tag:inbox | sed s/^thread:// > EXPECTED
 test_ruby <<"EOF"
 @q = @db.query('tag:inbox')
 @q.sort = Notmuch::SORT_OLDEST_FIRST
@@ -26,10 +28,9 @@ for t in @q.search_threads do
   print t.thread_id, "\n"
 end
 EOF
-notmuch search --sort=oldest-first --output=threads tag:inbox | sed s/^thread:// > EXPECTED
-test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "compare message ids"
+notmuch search --sort=oldest-first --output=messages tag:inbox | sed s/^id:// > EXPECTED
 test_ruby <<"EOF"
 @q = @db.query('tag:inbox')
 @q.sort = Notmuch::SORT_OLDEST_FIRST
@@ -37,40 +38,35 @@ for m in @q.search_messages do
   print m.message_id, "\n"
 end
 EOF
-notmuch search --sort=oldest-first --output=messages tag:inbox | sed s/^id:// > EXPECTED
-test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "get non-existent file"
+echo -n true > EXPECTED
 test_ruby <<"EOF"
 result = @db.find_message_by_filename('i-dont-exist')
 print (result == nil)
 EOF
-test_expect_equal "$(cat OUTPUT)" "true"
 
 test_begin_subtest "count messages"
+notmuch count --output=messages tag:inbox > EXPECTED
 test_ruby <<"EOF"
 @q = @db.query('tag:inbox')
 print @q.count_messages(),"\n"
 EOF
-notmuch count --output=messages tag:inbox > EXPECTED
-test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "count threads"
+notmuch count --output=threads tag:inbox > EXPECTED
 test_ruby <<"EOF"
 @q = @db.query('tag:inbox')
 print @q.count_threads(),"\n"
 EOF
-notmuch count --output=threads tag:inbox > EXPECTED
-test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "get all tags"
+notmuch search --output=tags '*' > EXPECTED
 test_ruby <<"EOF"
 @t = @db.all_tags()
 for tag in @t do
    print tag,"\n"
 end
 EOF
-notmuch search --output=tags '*' > EXPECTED
-test_expect_equal_file EXPECTED OUTPUT
 
 test_done
--
2.31.0
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
Felipe Contreras Felipe Contreras
Reply | Threaded
Open this post in threaded view
|

[PATCH 6/9] test: ruby: use much more standard puts

In reply to this post by Felipe Contreras
Signed-off-by: Felipe Contreras <[hidden email]>
---
 test/T395-ruby.sh | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/test/T395-ruby.sh b/test/T395-ruby.sh
index 55bf4c2b..f871ddd9 100755
--- a/test/T395-ruby.sh
+++ b/test/T395-ruby.sh
@@ -25,7 +25,7 @@ test_ruby <<"EOF"
 @q = @db.query('tag:inbox')
 @q.sort = Notmuch::SORT_OLDEST_FIRST
 for t in @q.search_threads do
-  print t.thread_id, "\n"
+  puts t.thread_id
 end
 EOF
 
@@ -35,29 +35,29 @@ test_ruby <<"EOF"
 @q = @db.query('tag:inbox')
 @q.sort = Notmuch::SORT_OLDEST_FIRST
 for m in @q.search_messages do
-  print m.message_id, "\n"
+  puts m.message_id
 end
 EOF
 
 test_begin_subtest "get non-existent file"
-echo -n true > EXPECTED
+echo true > EXPECTED
 test_ruby <<"EOF"
 result = @db.find_message_by_filename('i-dont-exist')
-print (result == nil)
+puts (result == nil)
 EOF
 
 test_begin_subtest "count messages"
 notmuch count --output=messages tag:inbox > EXPECTED
 test_ruby <<"EOF"
 @q = @db.query('tag:inbox')
-print @q.count_messages(),"\n"
+puts @q.count_messages()
 EOF
 
 test_begin_subtest "count threads"
 notmuch count --output=threads tag:inbox > EXPECTED
 test_ruby <<"EOF"
 @q = @db.query('tag:inbox')
-print @q.count_threads(),"\n"
+puts @q.count_threads()
 EOF
 
 test_begin_subtest "get all tags"
@@ -65,7 +65,7 @@ notmuch search --output=tags '*' > EXPECTED
 test_ruby <<"EOF"
 @t = @db.all_tags()
 for tag in @t do
-   print tag,"\n"
+   puts tag
 end
 EOF
 
--
2.31.0
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
Felipe Contreras Felipe Contreras
Reply | Threaded
Open this post in threaded view
|

[PATCH 7/9] test: ruby: use much more standard Ruby idioms

In reply to this post by Felipe Contreras
Signed-off-by: Felipe Contreras <[hidden email]>
---
 test/T395-ruby.sh | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/test/T395-ruby.sh b/test/T395-ruby.sh
index f871ddd9..f5a8d245 100755
--- a/test/T395-ruby.sh
+++ b/test/T395-ruby.sh
@@ -24,7 +24,7 @@ notmuch search --sort=oldest-first --output=threads tag:inbox | sed s/^thread://
 test_ruby <<"EOF"
 @q = @db.query('tag:inbox')
 @q.sort = Notmuch::SORT_OLDEST_FIRST
-for t in @q.search_threads do
+@q.search_threads.each do |t|
   puts t.thread_id
 end
 EOF
@@ -34,38 +34,34 @@ notmuch search --sort=oldest-first --output=messages tag:inbox | sed s/^id:// >
 test_ruby <<"EOF"
 @q = @db.query('tag:inbox')
 @q.sort = Notmuch::SORT_OLDEST_FIRST
-for m in @q.search_messages do
+@q.search_messages.each do |m|
   puts m.message_id
 end
 EOF
 
 test_begin_subtest "get non-existent file"
-echo true > EXPECTED
+echo nil > EXPECTED
 test_ruby <<"EOF"
-result = @db.find_message_by_filename('i-dont-exist')
-puts (result == nil)
+p @db.find_message_by_filename('i-dont-exist')
 EOF
 
 test_begin_subtest "count messages"
 notmuch count --output=messages tag:inbox > EXPECTED
 test_ruby <<"EOF"
-@q = @db.query('tag:inbox')
-puts @q.count_messages()
+puts @db.query('tag:inbox').count_messages()
 EOF
 
 test_begin_subtest "count threads"
 notmuch count --output=threads tag:inbox > EXPECTED
 test_ruby <<"EOF"
-@q = @db.query('tag:inbox')
-puts @q.count_threads()
+puts @db.query('tag:inbox').count_threads()
 EOF
 
 test_begin_subtest "get all tags"
 notmuch search --output=tags '*' > EXPECTED
 test_ruby <<"EOF"
-@t = @db.all_tags()
-for tag in @t do
-   puts tag
+@db.all_tags.each do |tag|
+  puts tag
 end
 EOF
 
--
2.31.0
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
Felipe Contreras Felipe Contreras
Reply | Threaded
Open this post in threaded view
|

[PATCH 8/9] test: ruby: don't use instance variables

In reply to this post by Felipe Contreras
Local variables are perfectly fine.

Signed-off-by: Felipe Contreras <[hidden email]>
---
 test/T395-ruby.sh | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/test/T395-ruby.sh b/test/T395-ruby.sh
index f5a8d245..30168109 100755
--- a/test/T395-ruby.sh
+++ b/test/T395-ruby.sh
@@ -12,7 +12,7 @@ test_ruby() {
     (
  cat <<-EOF
  require 'notmuch'
- @db = Notmuch::Database.new('$MAIL_DIR')
+ db = Notmuch::Database.new('$MAIL_DIR')
  EOF
  cat
     ) | $NOTMUCH_RUBY -I "$NOTMUCH_BUILDDIR/bindings/ruby"> OUTPUT
@@ -22,9 +22,9 @@ test_ruby() {
 test_begin_subtest "compare thread ids"
 notmuch search --sort=oldest-first --output=threads tag:inbox | sed s/^thread:// > EXPECTED
 test_ruby <<"EOF"
-@q = @db.query('tag:inbox')
-@q.sort = Notmuch::SORT_OLDEST_FIRST
-@q.search_threads.each do |t|
+q = db.query('tag:inbox')
+q.sort = Notmuch::SORT_OLDEST_FIRST
+q.search_threads.each do |t|
   puts t.thread_id
 end
 EOF
@@ -32,9 +32,9 @@ EOF
 test_begin_subtest "compare message ids"
 notmuch search --sort=oldest-first --output=messages tag:inbox | sed s/^id:// > EXPECTED
 test_ruby <<"EOF"
-@q = @db.query('tag:inbox')
-@q.sort = Notmuch::SORT_OLDEST_FIRST
-@q.search_messages.each do |m|
+q = db.query('tag:inbox')
+q.sort = Notmuch::SORT_OLDEST_FIRST
+q.search_messages.each do |m|
   puts m.message_id
 end
 EOF
@@ -42,25 +42,25 @@ EOF
 test_begin_subtest "get non-existent file"
 echo nil > EXPECTED
 test_ruby <<"EOF"
-p @db.find_message_by_filename('i-dont-exist')
+p db.find_message_by_filename('i-dont-exist')
 EOF
 
 test_begin_subtest "count messages"
 notmuch count --output=messages tag:inbox > EXPECTED
 test_ruby <<"EOF"
-puts @db.query('tag:inbox').count_messages()
+puts db.query('tag:inbox').count_messages()
 EOF
 
 test_begin_subtest "count threads"
 notmuch count --output=threads tag:inbox > EXPECTED
 test_ruby <<"EOF"
-puts @db.query('tag:inbox').count_threads()
+puts db.query('tag:inbox').count_threads()
 EOF
 
 test_begin_subtest "get all tags"
 notmuch search --output=tags '*' > EXPECTED
 test_ruby <<"EOF"
-@db.all_tags.each do |tag|
+db.all_tags.each do |tag|
   puts tag
 end
 EOF
--
2.31.0
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
Felipe Contreras Felipe Contreras
Reply | Threaded
Open this post in threaded view
|

[PATCH 9/9] test: ruby: simplify output comparison

In reply to this post by Felipe Contreras
Signed-off-by: Felipe Contreras <[hidden email]>
---
 test/T395-ruby.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/T395-ruby.sh b/test/T395-ruby.sh
index 30168109..597330d3 100755
--- a/test/T395-ruby.sh
+++ b/test/T395-ruby.sh
@@ -20,22 +20,22 @@ test_ruby() {
 }
 
 test_begin_subtest "compare thread ids"
-notmuch search --sort=oldest-first --output=threads tag:inbox | sed s/^thread:// > EXPECTED
+notmuch search --sort=oldest-first --output=threads tag:inbox > EXPECTED
 test_ruby <<"EOF"
 q = db.query('tag:inbox')
 q.sort = Notmuch::SORT_OLDEST_FIRST
 q.search_threads.each do |t|
-  puts t.thread_id
+  puts 'thread:%s' % t.thread_id
 end
 EOF
 
 test_begin_subtest "compare message ids"
-notmuch search --sort=oldest-first --output=messages tag:inbox | sed s/^id:// > EXPECTED
+notmuch search --sort=oldest-first --output=messages tag:inbox > EXPECTED
 test_ruby <<"EOF"
 q = db.query('tag:inbox')
 q.sort = Notmuch::SORT_OLDEST_FIRST
 q.search_messages.each do |m|
-  puts m.message_id
+  puts 'id:%s' % m.message_id
 end
 EOF
 
--
2.31.0
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/9] test: move test_ruby() inside the only client

In reply to this post by Felipe Contreras
Felipe Contreras <[hidden email]> writes:

> Not much point in polluting the main library, and also will be useful to
> modify it in tandem with the tests.
>

I can live with this change.

d
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/9] test: ruby: refactor test_ruby()

In reply to this post by Felipe Contreras
Felipe Contreras <[hidden email]> writes:

> There's no point in repeating the same initialization in all the tests.
>

LGTM

d
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 3/9] test: ruby: simplify MAIL_DIR check

In reply to this post by Felipe Contreras

LGTM. Although I am no expert on ruby idiom.

d
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/9] test: ruby: simplify MAIL_DIR initialization

In reply to this post by Felipe Contreras
Felipe Contreras <[hidden email]> writes:

> There's no need to complicate the script passing the MAIL_DIR
> environment variable.
>
> Signed-off-by: Felipe Contreras <[hidden email]>

The interpolation inside single quotes is slightly surprising, but OK

d
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 5/9] test: ruby: simplify test_ruby()

In reply to this post by Felipe Contreras
Felipe Contreras <[hidden email]> writes:

> We always do test_expect_equal_file, so do it in test_ruby() directly.
>
> The only subtest where we don't (get non-existent file) can be easily
> modified.

I'm slightly hesitent since every other test ends with test_expect_*.
OTOH it is self contained within this one file so I guess I can live
with this change.

d
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 6/9] test: ruby: use much more standard puts

In reply to this post by Felipe Contreras

No objection.

d
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 7/9] test: ruby: use much more standard Ruby idioms

In reply to this post by Felipe Contreras
Felipe Contreras <[hidden email]> writes:

> Signed-off-by: Felipe Contreras <[hidden email]>
> ---
>  test/T395-ruby.sh | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/test/T395-ruby.sh b/test/T395-ruby.sh
> index f871ddd9..f5a8d245 100755
> --- a/test/T395-ruby.sh
> +++ b/test/T395-ruby.sh
> @@ -24,7 +24,7 @@ notmuch search --sort=oldest-first --output=threads tag:inbox | sed s/^thread://
>  test_ruby <<"EOF"
>  @q = @db.query('tag:inbox')
>  @q.sort = Notmuch::SORT_OLDEST_FIRST
> -for t in @q.search_threads do
> +@q.search_threads.each do |t|
>    puts t.thread_id
>  end
>  EOF

The downside to these changes is that they make the tests harder for the
non-rubyist (i.e. me) to read. So I'm not (yet) convinced this is a good change.
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 8/9] test: ruby: don't use instance variables

In reply to this post by Felipe Contreras
Felipe Contreras <[hidden email]> writes:

> Local variables are perfectly fine.
>

LGTM (given it works)
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 9/9] test: ruby: simplify output comparison

In reply to this post by Felipe Contreras

I can live with this change.

d

_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/9] test: ruby: several cleanups and simplifications

In reply to this post by Felipe Contreras
Felipe Contreras <[hidden email]> writes:

> I found a lot of areas of improvement in the Ruby tests, so I decided to clean them up.
>
> With these changes the tests are now much simpler and follow more closely the typical Ruby idioms.
>
> Felipe Contreras (9):
>   test: move test_ruby() inside the only client
>   test: ruby: refactor test_ruby()
>   test: ruby: simplify MAIL_DIR check
>   test: ruby: simplify MAIL_DIR initialization
>   test: ruby: simplify test_ruby()
>   test: ruby: use much more standard puts

I have applied the first 6 patches to master.

d
_______________________________________________
notmuch mailing list -- [hidden email]
To unsubscribe send an email to [hidden email]
12