waterlink / spec2.cr

Enhanced `spec` testing library for [Crystal](http://crystal-lang.org/).
MIT License
103 stars 22 forks source link

Spec2 + WebMocks #31

Closed marceloboeira closed 7 years ago

marceloboeira commented 8 years ago

For some reason it is not possible to use Spec2 with Webmocks Eg.:

require "./spec_helper"

Spec2.describe My::Spec do
  let(response) { "example" }

  before do
    WebMock.stub(:get, "http://google.com").to_return(response)
  end

  it "tests" do
    ...
  end
end

Generates:

          (WebMock.stub(:get, "http://google.com")).to_return(response)
                   ^~~~

instantiating 'WebMock:Module#stub(Symbol, String)'
in ./libs/webmock/webmock.cr:16: undefined method 'stub' for Nil

    @@registry.stub(method, uri)
waterlink commented 8 years ago

No idea how it is possible. It is easily reproducible with this code, instead of webmock:

require "./spec_helper"

module Something
  @@some = 37

  def self.answer
    @@some + 5
  end
end

Spec2.describe "whatever" do
  it "works" do
    Something.answer
  end
end
          Something.answer
                    ^~~~~~

instantiating 'Something:Module#answer()'
in ./spec/spec2-issue-31_spec.cr:7: undefined method '+' for Nil

    @@some + 5
           ^
marceloboeira commented 8 years ago

@waterlink so, the problem is related to Module/Class variables?

waterlink commented 8 years ago

Yes. It seems, that the class variables are considered non-nillable only after the point when they are actually assigned a value.

waterlink commented 8 years ago

And all spec2 generated code happens to be defined before compiler considers this to happen

waterlink commented 8 years ago

Doing require "webmock" before require "./spec_helper" solves the problem :D

waterlink commented 8 years ago

I see. It fails right after the macro expansion is completed for the it "..." do ... end macro. At this stage, compiler probably still didn't even consider any @@class_variable = value assignments, so all @@class_variable-s have SomeType?, i.e.: nillable, so the code inside of the ParticularExample#run can not be compiled.

/cc @asterite WDYT about that, maybe there is something compiler can do better here?

asterite commented 8 years ago

@waterlink I'll need to see the generated code. This works (using std's spec):

require "spec"

module Something
  @@some = 37

  def self.answer
    @@some + 5
  end
end

describe "something" do
  it "works" do
    Something.answer
  end
end

I don't quite understand why in the case of spec2 it doesn't.

(there's a separate issue that class variables should be inherited, but they aren't right now)

waterlink commented 8 years ago

@asterite Is it possible to somehow output the code after full macros expansion? Because the code that is generated is not simple..

waterlink commented 8 years ago

Result of expansion (not full) of describe:

  1.       
  2.       
  3. 
  4.       
  5. 
  6.       __temp_103 = @@__spec2_active_context
  7.       module Spec2__Something
  8.         include ::Spec2::DSL::Spec2___
  9. 
 10.         
 11.           SPEC2_FULL_CONTEXT = ":root -> something (/home/oleksii/code/src/github.com/waterlink/spec2-issue-31/spec/spec2-issue-31_spec.cr:4)"
 12.           SPEC2_CONTEXT = Spec2__Something
 13.           LETS = {} of String => Int32
 14.           LETS_BANG = {} of String => Int32
 15.           BEFORES = [] of Int32
 16.           AFTERS = [] of Int32
 17.           ITS = {} of String => Int32
 18.           
 19.         
 20. 
 21.         __spec2_sanity_checks(Spec2__Something, ":root -> something (/home/oleksii/code/src/github.com/waterlink/spec2-issue-31/spec/spec2-issue-31_spec.cr:4)")
 22. 
 23.         @@__spec2_active_context = ::Spec2::Context
 24.           .new(__temp_103, "something")
 25. 
 26.         (__temp_103 ||
 27.          ::Spec2::Context.instance)
 28.           .contexts << @@__spec2_active_context
 29. 
 30.         it("tests") do
 31.   response = "example"
 32.   (WebMock.stub(:get, "http://google.com")).to_return(response)
 33. end
 34. blabla
 35. 
 36. 
 37.         __spec2_def_lets
 38.         __spec2_def_hooks
 39.         __spec2_def_its
 40.       end
 41. 

And result of expansion of it:

  1.       
  2. 
  3.       
  4.       
  5. 
  6.       class Spec2__Tests < ::Spec2::Example
  7.         include Spec2__Something
  8. 
  9.         def initialize(@context)
 10.           @what = "tests"
 11.           @blk = -> {}
 12.         end
 13. 
 14.         def run
 15.           __spec2_delayed = [] of ->
 16. 
 17.           __spec2_before_hook
 18.           __spec2_run_lets!
 19.           response = "example"
 20. (WebMock.stub(:get, "http://google.com")).to_return(response)
 21. blabla
 22. 
 23.           __spec2_after_hook
 24. 
 25.           __spec2_delayed.each &.call
 26.         end
 27.       end
 28. 
 29.       __temp_104 = (@@__spec2_active_context ||
 30.                           ::Spec2::Context.instance)
 31.       __temp_104
 32.         .examples << Spec2__Tests.new(__temp_104)
 33.    

I have achieved that by inserting a compilation error in one block or another in turns.

waterlink commented 8 years ago

As you can see

 31.       __temp_104
 32.         .examples << Spec2__Tests.new(__temp_104)

instantiation of the example happens inside of the module, that is enclosing this example (i.e.: outer describe module Spec2__Something). This might be relevant.

waterlink commented 8 years ago

On the side note: it would be nice to have a tool, that only expands macros and does nothing else and spits the macro-less crystal code :)

waterlink commented 8 years ago

I suppose this tool can already be built, from what compiler has internally already..

asterite commented 8 years ago

@waterlink You can put {{debug()}} (link) at the end of the macro to see what the macro generates.

asterite commented 8 years ago

But it's strange... @@some = 37 should be processed before the describe macro expansions. Can you try to show the problem without spec2? Maybe something like "If I initialize the module variable here, but later I have a code that does this and that, the compiler think the variable isn't initialized anymore".

waterlink commented 8 years ago

@asterite, unfortunately {{debug()}} does not do expansion of macro calls, that are found in the current macros body..

waterlink commented 8 years ago

@asterite I was able to nail it down to a very small example (without webmock and spec2, and even spec):

abstract class Example
  abstract def run
end

EXAMPLES = [] of Example

macro it(name, &block)
  class {{name.id}} < Example
    def run
      {{block.body}}
    end
  end

  EXAMPLES << {{name.id}}.new
end

at_exit do
  EXAMPLES.each(&.run)
end

class Universe
  @@value = 37
  def self.answer
    @@value + 5
  end
end

it(Something) do
  pp Universe.answer
end

Error is here:

Error in ./spec/spec2-issue-31_spec.cr:18: instantiating 'Array(Example+)#each()'

  EXAMPLES.each(&.run)
           ^~~~

in /opt/crystal/src/array.cr:776: instantiating 'each_index()'

    each_index do |i|
    ^~~~~~~~~~

in /opt/crystal/src/array.cr:776: instantiating 'each_index()'

    each_index do |i|
    ^~~~~~~~~~

in ./spec/spec2-issue-31_spec.cr:18: instantiating 'Array(Example+)#each()'

  EXAMPLES.each(&.run)
           ^~~~

in ./spec/spec2-issue-31_spec.cr:18: instantiating 'Example+#run()'

  EXAMPLES.each(&.run)
                  ^~~

in macro 'pp' /opt/crystal/src/macros.cr:66, line 2:

  1.   
  2.     ::puts "#{ "Universe.answer" } = #{ (Universe.answer).inspect }"
  3.   

    ::puts "#{ "Universe.answer" } = #{ (Universe.answer).inspect }"
                                                  ^~~~~~

instantiating 'Universe:Class#answer()'
in ./spec/spec2-issue-31_spec.cr:24: undefined method '+' for Nil

    @@value + 5
            ^

Moving at_exit after the definition of the Universe class solves the problem. So the problem is probably at_exit block, that tries to resolve all the types at the moment of encounter.

waterlink commented 8 years ago

https://carc.in/#/r/u50

asterite commented 8 years ago

I understand the problem now. It's basically this:

# This is a stub for at_exit
def foo(&block)
  block
end

# Compiler doesn't know when this callback will be executed,
# but it analyzes the body, see it calls Universe.answer, and types
# it, *before* @@value was initialized
foo do
  Universe.answer
end

class Universe
  @@value = 37

  def self.answer
    @@value + 5
  end
end

The main issue is that you can also do:

a = 1

class Universe
  @@value = a
end

Here, we assign the value of a local variable. That means that the code runs from top to bottom and when it reaches @@value = a, just there it initializes it. So using @@value before that line makes it nilable.

Maybe class variables should be initialized in a similar way to how constants are initialized, before the "main" code (but, of course, you won't be able to use local variables for that). It's something we definitely need to improve.

As a workaround for now, we can probably make @@registry in webmock a constant.

waterlink commented 8 years ago

What about a hook into a compilation semantics? Like at_exit is a hook into runtime semantics, something like at_compile_finish:

# .. define APIs here for testing framework ..

# define at_exit, but only when compiler finished compiling everything, right before it starts to do code generation:
at_compile_finish do
  at_exit do
    TestFramework.run
  end
end

# everything after this will be actually compiled before `at_compile_finish` block.
waterlink commented 8 years ago

Maybe it is too specific to add as a feature to the language, though..

marceloboeira commented 8 years ago

@astrite / @waterlink taking the opportunity, I wonder if it would be nice to start migrating/integrating the features form Spec2 to the std. What do you guys think? (I mess let statements, expect syntax ...)

waterlink commented 8 years ago

@marceloboeira I don't think it is a good idea. Reasons are following:

I don't think it is possible to deal with the first problem, it is simply a nature of this test framework.

Second problem, though can be dealt with, at compiler level, by either:

marceloboeira commented 8 years ago

@waterlink understood!

asterite commented 8 years ago

@waterlink We thought many times with @waj about an at_exit for macros. The problem is that this generates more code... what happens with multiple at_exit? They affect the new generated code? After you generate code you run it again? It's complex and probably hard to understand. I guess class variables initializers should happen at the begining of the program (probably), so it's something we need to change (or better, rethink) in the language.

waterlink commented 7 years ago

@asterite I understand these concerns. Did anything change with how class variable initializers work in the meantime?

@marceloboeira Do you still encounter this problem today?

asterite commented 7 years ago

@waterlink Yes! https://github.com/crystal-lang/crystal/pull/3612

theodorton commented 7 years ago

FYI - No issues using Spec2 and WebMock. I believe this issue can be closed.