Skip to content

Added a failing test to show that mocking File.read_lines is currently broken#41

Open
ndbroadbent wants to merge 4 commits intowaterlink:masterfrom
DocSpring:failing_test_for_file_readlines
Open

Added a failing test to show that mocking File.read_lines is currently broken#41
ndbroadbent wants to merge 4 commits intowaterlink:masterfrom
DocSpring:failing_test_for_file_readlines

Conversation

@ndbroadbent
Copy link

@ndbroadbent ndbroadbent commented Dec 6, 2019

This PR includes #40, which fixes most of the tests that were crashing on Crystal 0.31.1

This related spec is still failing: #37

So I think I'm running into the same issue with this case.

See also: #38

@ndbroadbent ndbroadbent changed the title Added a failing test to show that mocking File.readlines is currently broken Added a failing test to show that mocking File.read_lines is currently broken Dec 6, 2019
@ndbroadbent
Copy link
Author

ndbroadbent commented Dec 6, 2019

I did some digging in the Crystal stdlib and figured out that File.exists? actually calls a OS specific method:

Both of these define a Crystal::System::File module with self.exists?.

So I changed the test to:


Mocks.create_module_mock Crystal::System::File do
  mock self.exists?(path)
  mock self.read_lines(filename, encoding = nil, invalid = nil, chomp = true)
end

And this fixed the original exists? test:

Mocks.create_module_mock Crystal::System::File do
  mock self.exists?(path)
end

describe "create module mock macro" do
  it "does not fail with Nil errors" do
    allow(MyModule).to receive(self.exists?("hello")).and_return(true)
    MyModule.exists?("world").should eq(false)
    MyModule.exists?("hello").should eq(true)
  end

  it "does not fail with Nil errors for stdlib class" do
    allow(Crystal::System::File).to receive(self.exists?("hello")).and_return(true)
    File.exists?("world").should eq(false)
    File.exists?("hello").should eq(true)
  end
end

So it looks like there could be two ways of solving this:

  • Transparently handle the File case inside mocks.cr, so that the user doesn't need to think about the implementation details
  • Add a note about this to the README

(I would prefer the first option if that's possible.)

I'm still working on the File.read_lines mock, because I can't get that to work:


require "../spec_helper"

Mocks.create_mock File do
  mock self.read_lines(filename, encoding = nil, invalid = nil, chomp = true)
end

describe "mocking File calls" do
  it "mocks File.read_lines" do
    allow(File).to receive(self.read_lines("example")).and_return(["hey, world!"])
    File.read_lines("example").should eq(["hey, world!"])
  end
end

But it's surprising that this is so hard, because it's just a simple method defined on the File class. (But I'm doing something wrong!)

Test currently fails because the method isn't mocked properly, so it tries to read a file that doesn't exist:

  1) mocking File.read_lines

       Error opening file 'example' with mode 'r': No such file or directory (Errno)
         from /usr/local/Cellar/crystal/0.31.1/src/crystal/system/unix/file.cr:10:7 in 'open'
         from /usr/local/Cellar/crystal/0.31.1/src/file.cr:105:5 in 'new'
         from /usr/local/Cellar/crystal/0.31.1/src/file.cr:596:5 in 'read_lines'
         ...

P.S. The crystal tool expand command is pretty awesome! Here's the expanded macro for the Mocks.create_mock File line:

$ crystal tool expand -c spec/mocks/file_mocks_spec.cr:28:1 spec/mocks/file_mocks_spec.cr
1 expansion found
expansion 1:
   Mocks.create_mock(File) do
     mock(self.read_lines(filename, encoding = nil, invalid = nil, chomp = true))
   end

# expand macro 'Mocks.create_mock' (/Users/ndbroadbent/code/mocks.cr/src/macro/create_mock.cr:3:5)
~> class ::File
     @@__mocks_name = "File"
     include ::Mocks::BaseMock
     mock(self.read_lines(filename, encoding = nil, invalid = nil, chomp = true))
   end
   class ::Mocks::InstanceDoublesFile < ::Mocks::BaseDouble
     @@name = "Mocks::InstanceDoublesFile"
     def ==(other)
       self.same?(other)
     end
     def ==(other : Value)
       false
     end
     macro mock(method_spec, flag = :normal)
             _mock({{ method_spec }}, nil, ::File.allocate)
             end
     mock(self.read_lines(filename, encoding = nil, invalid = nil, chomp = true))
   end

# expand macro 'mock' (/Users/ndbroadbent/code/mocks.cr/src/macro/base_mock.cr:3:5)
# expand macro 'mock' (/Users/ndbroadbent/code/mocks.cr/spec/mocks/file_mocks_spec.cr:28:1)
~> class ::File
     @@__mocks_name = "File"
     include ::Mocks::BaseMock
       # Returns all lines in *filename* as an array of strings.
     #
     # ```
     # File.write("foobar", "foo\nbar")
     # File.read_lines("foobar") # => ["foo", "bar"]
     # ```
   def self.read_lines(filename, encoding = nil, invalid = nil, chomp = true)
       __temp_28 = @@__mocks_name
       if __temp_28
       else
         raise("Assertion failed (mocks.cr): @@__mocks_name can not be nil")
       end
       ::Mocks::Registry.remember(typeof({filename, encoding = nil, invalid = nil, chomp = true}))
       __temp_29 = (::Mocks::Registry(typeof({filename, encoding = nil, invalid = nil, chomp = true})).for(__temp_28)).fetch_method("self.read_lines")
       __temp_30 = __temp_29.call(::Mocks::Registry::ObjectId.build(self), {filename, encoding = nil, invalid = nil, chomp = true})
       if __temp_30.call_original
         previous_def(filename, encoding, invalid, chomp)
       else
         if __temp_30.value.is_a?(typeof(previous_def(filename, encoding, invalid, chomp)))
           __temp_30.value.as(typeof(previous_def(filename, encoding, invalid, chomp)))
         else
           __temp_31 = "#{self.inspect} attempted to return stubbed value of wrong type, while calling"
           __temp_32 = "Expected type: #{typeof(previous_def(filename, encoding, invalid, chomp))}. Actual type: #{__temp_30.value.class}"
           raise(::Mocks::UnexpectedMethodCall.new("#{__temp_31} self.read_lines#{[filename, encoding = nil, invalid = nil, chomp = true]}. #{__temp_32}"))
         end
       end
     end
   end
   class ::Mocks::InstanceDoublesFile < ::Mocks::BaseDouble
     @@name = "Mocks::InstanceDoublesFile"
     def ==(other)
       self.same?(other)
     end
     def ==(other : Value)
       false
     end
     macro mock(method_spec, flag = :normal)
             _mock({{ method_spec }}, nil, ::File.allocate)
             end
     _mock(self.read_lines(filename, encoding = nil, invalid = nil, chomp = true), nil, ::File.allocate)
   end

# expand macro '_mock' (/Users/ndbroadbent/code/mocks.cr/src/macro/base_double.cr:3:5)
~> class ::File
     @@__mocks_name = "File"
     include ::Mocks::BaseMock
       # Returns all lines in *filename* as an array of strings.
     #
     # ```
     # File.write("foobar", "foo\nbar")
     # File.read_lines("foobar") # => ["foo", "bar"]
     # ```
   def self.read_lines(filename, encoding = nil, invalid = nil, chomp = true)
       __temp_28 = @@__mocks_name
       if __temp_28
       else
         raise("Assertion failed (mocks.cr): @@__mocks_name can not be nil")
       end
       ::Mocks::Registry.remember(typeof({filename, encoding = nil, invalid = nil, chomp = true}))
       __temp_29 = (::Mocks::Registry(typeof({filename, encoding = nil, invalid = nil, chomp = true})).for(__temp_28)).fetch_method("self.read_lines")
       __temp_30 = __temp_29.call(::Mocks::Registry::ObjectId.build(self), {filename, encoding = nil, invalid = nil, chomp = true})
       if __temp_30.call_original
         previous_def(filename, encoding, invalid, chomp)
       else
         if __temp_30.value.is_a?(typeof(previous_def(filename, encoding, invalid, chomp)))
           __temp_30.value.as(typeof(previous_def(filename, encoding, invalid, chomp)))
         else
           __temp_31 = "#{self.inspect} attempted to return stubbed value of wrong type, while calling"
           __temp_32 = "Expected type: #{typeof(previous_def(filename, encoding, invalid, chomp))}. Actual type: #{__temp_30.value.class}"
           raise(::Mocks::UnexpectedMethodCall.new("#{__temp_31} self.read_lines#{[filename, encoding = nil, invalid = nil, chomp = true]}. #{__temp_32}"))
         end
       end
     end
   end
   class ::Mocks::InstanceDoublesFile < ::Mocks::BaseDouble
     @@name = "Mocks::InstanceDoublesFile"
     def ==(other)
       self.same?(other)
     end
     def ==(other : Value)
       false
     end
     macro mock(method_spec, flag = :normal)
             _mock({{ method_spec }}, nil, ::File.allocate)
             end
     def self.read_lines(filename, encoding = nil, invalid = nil, chomp = true)
       ::Mocks::Registry.remember(typeof({filename, encoding = nil, invalid = nil, chomp = true}))
       __temp_33 = (::Mocks::Registry(typeof({filename, encoding = nil, invalid = nil, chomp = true})).for(@@name)).fetch_method("self.read_lines")
       __temp_34 = __temp_33.call(::Mocks::Registry::ObjectId.build(self), {filename, encoding = nil, invalid = nil, chomp = true})
       if __temp_34.call_original
         raise(::Mocks::UnexpectedMethodCall.new("#{self.inspect} received unexpected method call self.read_lines#{[filename, encoding = nil, invalid = nil, chomp = true]}"))
       else
         if __temp_34.value.is_a?(typeof((typeof(::File.allocate)).read_lines(filename, encoding = nil, invalid = nil, chomp = true)))
           __temp_34.value.as(typeof((typeof(::File.allocate)).read_lines(filename, encoding = nil, invalid = nil, chomp = true)))
         else
           raise(::Mocks::UnexpectedMethodCall.new("#{self.inspect} received unexpected method call self.read_lines#{[filename, encoding = nil, invalid = nil, chomp = true]}"))
         end
       end
     end
   end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant