Merge pull request #124 from mvidner/ops-get

Fixed to report the right frame for Ops.get_foo (bnc#877758).

package/yast2-ruby-bindings.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Wed Jul 30 14:27:09 UTC 2014 - mvidner@suse.com
+
+- Fixed to report the right frame for Ops.get_foo (bnc#877758).
+- Futureproof tests for RSpec 3 even more, avoid be_true.
+- 3.1.23
+
 -------------------------------------------------------------------
 Wed Jul 23 17:18:58 CEST 2014 - locilka@suse.com
 

package/yast2-ruby-bindings.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-ruby-bindings
-Version:        3.1.22
+Version:        3.1.23
 Url:            https://github.com/yast/yast-ruby-bindings
 Release:        0
 BuildRoot:      %{_tmppath}/%{name}-%{version}-build

src/ruby/yast/ops.rb

@@ -45,27 +45,34 @@ module Ops
     ]
 
     Ops::SHORTCUT_TYPES.each do |type|
-      eval <<END
-        def self.get_#{type}(*args, &block)
-          Yast::Convert.to_#{type} get(*args, &block)
+      eval <<END, binding, __FILE__, __LINE__ + 1
+        def self.get_#{type}(object, indexes, default=nil, &block)
+          Yast::Convert.to_#{type} get(object, indexes, default, 1, &block)
         end
 END
     end
 
-    # to log outer frame we need to skip 3 frames as 1 is method itself and
-    # 2 frames generate each block. Try your self:
+    # To log the caller frame we need to skip 3 frames as 1 is method itself
+    # and each block contributes 2 frames (outer: called, inner: defined)
+    # Try for yourself:
     #   def a
     #     puts caller.inspect
     #     [0].each { |i| puts caller.inspect }
     #   end
     #   a
     OUTER_LOOP_FRAME = 3
 
-    # gets value from object at indexes. In case that value is not found, then return default value.
-    # @deprecated use ruby native operator []
-    def self.get (object, indexes, default=nil)
+    # @deprecated Use the native Ruby operator `[]`
+    #
+    # Gets value from *object* at *indexes*.
+    # In case value is not found, then return *default* value.
+    #
+    # @param skip_frames [Integer] private, how many caller frames to skip
+    #  when reporting warnings or exceptions (0 by default)
+    def self.get (object, indexes, default=nil, skip_frames = 0)
       res = object
       default = Yast.deep_copy(default)
+      skip_frames += OUTER_LOOP_FRAME
       indexes = [indexes] unless indexes.is_a? ::Array
 
       indexes.each do |i|
@@ -75,11 +82,11 @@ def self.get (object, indexes, default=nil)
             if (0..res.size-1).include? i
               res = res[i]
             else
-              Yast.y2milestone OUTER_LOOP_FRAME, "Index #{i} is out of array size"
+              Yast.y2milestone skip_frames, "Index #{i} is out of array size"
               return block_given? ? yield : default
             end
           else
-            Yast.y2warning OUTER_LOOP_FRAME, "Passed #{i.inspect} as index key for array."
+            Yast.y2warning skip_frames, "Passed #{i.inspect} as index key for array."
             return block_given? ? yield : default
           end
         when ::Hash
@@ -89,10 +96,10 @@ def self.get (object, indexes, default=nil)
             return block_given? ? yield : default
           end
         when ::NilClass
-          Yast.y2milestone OUTER_LOOP_FRAME, "Builtin index called on nil."
+          Yast.y2milestone skip_frames, "Ops.get called on nil."
           return block_given? ? yield : default
         else
-          Yast.y2warning OUTER_LOOP_FRAME, "Builtin index called on wrong type #{res.class}"
+          Yast.y2warning skip_frames, "Ops.get called on wrong type #{res.class}"
           return block_given? ? yield : default
         end
       end

tests/ruby/builtins_spec.rb

@@ -232,7 +232,7 @@
   end
 
   it "tests time" do
-    expect(Yast::Builtins.time > 0).to be_true
+    expect(Yast::Builtins.time).to be > 0
   end
 
   it "tests find string" do
@@ -345,7 +345,7 @@
   end
 
   it "tests srandom" do
-    expect(Yast::Builtins.srandom() > 0).to be_true
+    expect(Yast::Builtins.srandom()).to be > 0
     expect(Yast::Builtins.srandom(10)).to eq(nil)
   end
 
@@ -742,7 +742,7 @@
 
     # there is quite nice chance with this repetition to test even border or range
     100.times do
-      expect((0..9).include? Yast::Builtins.random(10)).to be_true
+      expect(0..9).to cover Yast::Builtins.random(10)
     end
   end
 
@@ -838,7 +838,7 @@
     # crypt is salted so cannot reproduce, just test if run and returns something useful
     ["", "md5", "blowfish", "sha256", "sha512"].each do |suffix|
       res = Yast::Builtins.send(:"crypt#{suffix}", "test")
-      expect(res).to be_true
+      expect(res).to be_a String
       expect(res.size).to be > 10
     end
   end

tests/ruby/exportable_spec.rb

@@ -33,7 +33,6 @@ def test(a,b)
   end
 
   it "tests publish variables" do
-    expect(MyTest.class.published_variables[:variable_a]).to be_true
     expect(MyTest.class.published_variables[:variable_a].type).to eq("map<any,any>")
   end
 

tests/ruby/ops_spec.rb

@@ -117,48 +117,104 @@
     expect(Yast::Ops.less_than({"a" => 1, 1 => 2},{"a" => 1, "b" => 2})).to eq(true)
   end
 
-  it "tests get map" do
-    map = { "a" => { "b" => "c" }}
-    expect(Yast::Ops.get(map,"a","n")).to eq({ "b" => "c"})
-    expect(Yast::Ops.get(map,["a","b"],"n")).to eq("c")
-    expect(Yast::Ops.get(map,["a","c"],"n")).to eq("n")
-    expect(Yast::Ops.get(map,["c","b"],"n")).to eq("n")
-    expect(Yast::Ops.get(map,["c","b"]){ "n" }).to eq("n")
-  end
+  describe "Ops.get" do
+    context "when the container is a map" do
+      let(:map) { { "a" => { "b" => "c" }} }
+
+      it "returns value if key exists" do
+        expect(Yast::Ops.get(map,"a","n")).to eq({ "b" => "c"})
+      end
+
+      it "supports nested access with list of keys" do
+        expect(Yast::Ops.get(map,["a","b"],"n")).to eq("c")
+      end
+
+      it "returns default if any key is not available" do
+        expect(Yast::Ops.get(map,["a","c"],"n")).to eq("n")
+        expect(Yast::Ops.get(map,["c","b"],"n")).to eq("n")
+      end
+
+      it "support blocks for default where it return result of block" do
+        expect(Yast::Ops.get(map,["c","b"]){ "n" }).to eq("n")
+      end
+    end
 
-  it "tests get list" do
-    list = [["a","b"]]
-    expect(Yast::Ops.get(list,0,"n")).to eq(["a","b"])
-    expect(Yast::Ops.get(list,[0,1],"n")).to eq("b")
-    expect(Yast::Ops.get(list,[0,2],"n")).to eq("n")
-    expect(Yast::Ops.get(list,[1,1],"n")).to eq("n")
-  end
+    context "when the container is a list" do
+      let(:list)  { [["a","b"]] }
+      let(:list2) { ["a"] }
 
-  it "tests get term" do
-    term = Yast::Term.new(:a,"a","b")
-    expect(Yast::Ops.get(term,1,"n")).to eq("b")
-    expect(Yast::Ops.get(term,[2],"n")).to eq("n")
-  end
+      it "returns value if key exists" do
+        expect(Yast::Ops.get(list,0,"n")).to eq(["a","b"])
+      end
 
-  it "tests get mixture" do
-    map_list =  { "a" => ["b","c"]}
-    expect(Yast::Ops.get(map_list,["a",1],"n")).to eq("c")
-    expect(Yast::Ops.get(map_list,["a",2],"n")).to eq("n")
-    map_term =  { "a" => Yast::Term.new(:a,"b","c")}
-    expect(Yast::Ops.get(map_term,["a",1],"n")).to eq("c")
-    expect(Yast::Ops.get(map_term,["a",2],"n")).to eq("n")
-  end
+      it "supports nested access with list of keys" do
+        expect(Yast::Ops.get(list,[0,1],"n")).to eq("b")
+        expect(Yast::Ops.get(list,[0,2],"n")).to eq("n")
+        expect(Yast::Ops.get(list,[1,1],"n")).to eq("n")
+      end
+
+      it "returns default value when indexing with a non-integer" do
+        expect(Yast::Ops.get(list2,["a"],"n")).to eq("n")
+      end
+
+      it "returns default value for too many indices" do
+        expect(Yast::Ops.get(list2,[0,0],"n")).to eq("n")
+      end
+    end
+
+    context "when the container is a term" do
+      let(:term) { Yast::Term.new(:a,"a","b") }
+
+      it "returns value if key exists" do
+        expect(Yast::Ops.get(term,1,"n")).to eq("b")
+      end
+
+      it "returns default if FIXME" do
+        expect(Yast::Ops.get(term,[2],"n")).to eq("n")
+      end
+    end
 
-  it "tests get corner cases" do
-    list = ["a"]
-    expect(Yast::Ops.get(list,["a"],"n")).to eq("n")
-    expect(Yast::Ops.get(list,[0,0],"n")).to eq("n")
+    context "when the container is heterogeneous" do
+      let(:map_list) { { "a" => ["b","c"]} }
+      let(:map_term) { { "a" => Yast::Term.new(:a,"b","c")} }
+
+      it "supports nested access with list of keys" do
+        expect(Yast::Ops.get(map_list,["a",1],"n")).to eq("c")
+        expect(Yast::Ops.get(map_term,["a",1],"n")).to eq("c")
+      end
+
+      it "returns default if any key is not available" do
+        expect(Yast::Ops.get(map_list,["a",2],"n")).to eq("n")
+        expect(Yast::Ops.get(map_term,["a",2],"n")).to eq("n")
+      end
+    end
   end
 
-  it "tests get shortcuts" do
-    list = ["a","b"]
-    expect(Yast::Ops.get_string(list,0,"n")).to eq("a")
-    expect(Yast::Ops.get_integer(list,0,"n")).to eq(nil)
+  describe "Ops.get_foo shortcuts" do
+    let(:list) { ["a","b"] }
+
+    it "returns .get result for a matching type" do
+      expect(Yast::Ops.get_string(list,0,"n")).to eq("a")
+    end
+
+    it "returns nil for a mismatching type" do
+      expect(Yast::Ops.get_integer(list,0,"n")).to eq(nil)
+    end
+
+    it "warns when the container is nil" do
+      any_frame = kind_of(Integer)
+      expect(Yast).to receive(:y2milestone).with(any_frame, /called on nil/)
+      Yast::Ops.get_string(nil, 0, "n")
+    end
+
+    it "reports the right location when warning" do
+      # The internal method that sees the file is:
+      # y2_logger(log_level, component, file, line, method, format, args)
+      line = __LINE__ + 3 # this must be the line where get_string is called
+      expect(Yast).to receive(:y2_logger).
+        with(kind_of(Integer), "Ruby", __FILE__, line, //, //)
+      Yast::Ops.get_string(nil, 0, "n")
+    end
   end
 
   it "tests set" do
@@ -424,12 +480,12 @@
   end
 
   it "tests is" do
-    expect(Yast::Ops.is("t", "string")).to be_true
-    expect(!Yast::Ops.is("t", "integer")).to be_true
+    expect(Yast::Ops.is("t", "string")).to be true
+    expect(Yast::Ops.is("t", "integer")).to be false
   end
 
   it "tests is shortcut" do
-    expect(Yast::Ops.is_string?("t")).to be_true
-    expect(!Yast::Ops.is_void?("t")).to be_true
+    expect(Yast::Ops.is_string?("t")).to be true
+    expect(Yast::Ops.is_void?("t")).to be false
   end
 end