From: Ryan Davis <ryand-ruby@...>
Date: 2010-10-21T05:16:59+09:00
Subject: [ruby-core:32863] REVIEW: clean up test/unit assert

2 Changes:

1) I find having a default arg of (nomsg = true; nil) to be confusing and not an idiom we want to teach others. I've switched it to a much more self-documenting assignment of UNASSIGNED.

2) Nowhere (that I know of) do minitest's assertions pass a non-proc or non-string object up to assert, so all the additional logic of checking the backtrace is unnecessary. I also dislike assignments in conditionals so this removes it entirely.

Index: assertions.rb
===================================================================
--- assertions.rb	(revision 29541)
+++ assertions.rb	(working copy)
@@ -10,10 +10,12 @@
         obj.pretty_inspect.chomp
       end
 
-      def assert(test, msg = (nomsg = true; nil))
-        unless nomsg or msg.instance_of?(String) or msg.instance_of?(Proc) or
-            (bt = caller).first.rindex(MiniTest::MINI_DIR, 0)
-          bt.delete_if {|s| s.rindex(MiniTest::MINI_DIR, 0)}
+      UNASSIGNED = Object.new
+
+      def assert(test, msg = UNASSIGNED)
+        msg = nil if msg == UNASSIGNED
+        unless String === msg or Proc === msg then
+          bt = caller.reject { |s| s.rindex(MiniTest::MINI_DIR, 0) }
           raise ArgumentError, "assertion message must be String or Proc, but #{msg.class} was given.", bt
         end
         super