eiri commented on a change in pull request #560: Fix broken eunit test in 
changes_since_test_ test suite
URL: https://github.com/apache/couchdb/pull/560#discussion_r120667148
 
 

 ##########
 File path: src/couch_mrview/test/couch_mrview_changes_since_tests.erl
 ##########
 @@ -19,42 +19,79 @@
 
 
 
-setup() ->
-    {ok, Db} = couch_mrview_test_util:init_db(?tempdb(), changes),
+setup(Opts) ->
+    {ok, Db} = couch_mrview_test_util:init_db(
+        ?tempdb(),
+        {changes, Opts}
+    ),
     Db.
 
-teardown(Db) ->
+teardown(_, Db) ->
     couch_db:close(Db),
     couch_server:delete(Db#db.name, [?ADMIN_CTX]),
     ok.
 
 
-changes_since_test() ->
+changes_since_basic_test_() ->
     {
         "changes_since tests",
         {
             setup,
-            fun test_util:start_couch/0, fun test_util:stop_couch/1,
-            {
-                foreach,
-                fun setup/0, fun teardown/1,
-                [
-                    fun test_basic/1,
-                    fun test_range/1,
-                    fun test_basic_since/1,
-                    fun test_range_since/1,
-                    fun test_basic_count/1,
-                    fun test_range_count/1,
-                    fun test_basic_count_since/1,
-                    fun test_range_count_since/1,
-                    fun test_compact/1,
-                    fun test_remove_key/1
-                ]
-            }
+            fun test_util:start_couch/0, fun test_util:stop/1,
+            [
+                make_test_case(seq_indexed, [fun test_basic/2]),
+                make_test_case(seq_indexed, [fun test_basic_since/2]),
+                make_test_case(seq_indexed, [fun test_basic_count/2]),
+                make_test_case(seq_indexed, [fun test_basic_count_since/2]),
+                make_test_case(seq_indexed, [fun test_compact/2])
+            ]
         }
     }.
 
-test_basic(Db) ->
+
+changes_since_range_test_() ->
+    {
+        "changes_since_range tests",
+        {
+            setup,
+            fun test_util:start_couch/0, fun test_util:stop/1,
+            [
+                make_test_case(keyseq_indexed, [fun test_range/2]),
+                make_test_case(keyseq_indexed, [fun test_range_since/2]),
+                make_test_case(keyseq_indexed, [fun test_remove_key/2])
+            ]
+        }
+    }.
+
+
+changes_since_range_count_test_() ->
+    {
+        "changes_since_range_count test",
+        {
+            setup,
+            fun test_util:start_couch/0, fun test_util:stop/1,
+            [
+                make_test_case(
+                  seq_indexed_keyseq_indexed,
+                  [fun test_range_count/2]
+                ),
+                make_test_case(
+                  seq_indexed_keyseq_indexed,
+                  [fun test_range_count_since/2]
+                )
+            ]
+        }
+    }.
+
+
+make_test_case(Mod, Funs) ->
 
 Review comment:
   This is a step in a right direction, but it is rather over engineered. Our 
setup function is two lines, no need to make it genetic and invoked on 
foreachx. Just split `changes_since_test_` in two for all the since and all the 
range and define setup right on suite, something like this:
   
   ```
   changes_since_basic_test_() ->
       {
           "changes_since tests",
           {
               setup,
               fun test_util:start_couch/0, fun test_util:stop_couch/1,
               {
                   foreach,
                   fun() ->
                       Type = {changes, seq_indexed},
                       {ok, Db} = couch_mrview_test_util:init_db(?tempdb(), 
Type),
                       Db
                   end,
                   fun teardown/1,
                   [
                       fun test_basic/1,
                       fun test_basic_since/1,
                       fun test_basic_count/1,
                       fun test_basic_count_since/1,
                       fun test_compact/1
                   ]
               }
           }
       }.
   
   changes_since_range_test_() ->
       {
           "changes_since_range tests",
           {
               setup,
               fun test_util:start_couch/0, fun test_util:stop_couch/1,
               {
                   foreach,
                   fun() ->
                       Type = {changes, keyseq_indexed},
                       {ok, Db} = couch_mrview_test_util:init_db(?tempdb(), 
Type),
                       Db
                   end,
                   fun teardown/1,
                   [
                       fun test_range/1,
                       fun test_range_since/1,
                       fun test_remove_key/1
                   ]
               }
           }
       }.
   
   changes_since_range_count_test_() ->
       {
           "changes_since_range_count tests",
           {
               setup,
               fun test_util:start_couch/0, fun test_util:stop_couch/1,
               {
                   foreach,
                   fun() ->
                       Type = {changes, seq_indexed_keyseq_indexed},
                       {ok, Db} = couch_mrview_test_util:init_db(?tempdb(), 
Type),
                       Db
                   end,
                   fun teardown/1,
                   [
                       fun test_range_count/1,
                       fun test_range_count_since/1
                   ]
               }
           }
       }.
   ```
   
   I understand it looks less crafty, but it's better to keep tests as clean as 
possible, it's hard enough to figure out why tests failing without adding need 
to process how they actually work.
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to