waterlink / spec2.cr

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

Feature request: described_class #36

Closed justaskz closed 8 years ago

justaskz commented 8 years ago

Is described_class is in a roadmap?

https://www.relishapp.com/rspec/rspec-core/docs/metadata/described-class

waterlink commented 8 years ago

@justaskz Thanks! This is really interesting item to put on roadmap. Could you create a PR to README.md in the roadmap section with corresponding new todo item?

mauricioabreu commented 8 years ago

I think this issue can be closed. I cloned the repository to add it to the roadmap and it was already there. wow!

mauricioabreu commented 8 years ago

Hello @waterlink

Is this a hard feature to include? I am willing to add it to spec2 but I am having a hard time reading the codebase. I am new to Crystal (also new to Ruby in syntax terms) and I wanna contribute back to this project.

I already have the test:

class DescribedClass; end

describe DescribedClass do
  context "when describe uses a class" do
    it "has the described_class attribute" do
      expect(described_class).to eq(DescribedClass)
    end
  end
end

Sorry for taking your time and for commenting in an issue I said to close.

waterlink commented 8 years ago

Hi @mauricioabreu

Yep, it is hard to read code, mostly, because it is doing some crazy macro-magic to give the user nice DSL. Under the hood, it is actually just a hierarchical classes, that are inner classes related to their parent and they derive from it too.

If you read non-sugar/non-DSL parts of the library, it should actually be pretty good, except for a slightly higher bit of abstraction, that allows for custom reporters, runners, output formatters, etc.

Being oversimplified, de-sugarized code would look like (your example) this (roughly):

class DescribedClass; end

class DescribeDescribedClass < Spec2::RootContext
  # .. lets, subject, before and after here ..

  class ContextWhenDescribeUsesAClass < DescribeDescribedClass
    # .. lets, subject, before and after here ..

    class ItHasTheDescribedClassAttribute < ContextWhenDescribeUsesAClass
      include Spec2::Example

      def run
        # .. some stuff here (like all before and let! runs) ..
        expect(described_class).to eq(DescribedClass)
        # .. more stuff here (like all after and defered runs) ..
      end
    end
  end
end

Most of sugarizing is done here: https://github.com/waterlink/spec2.cr/blob/master/src/next/dsl.cr

To implement described_class, it might be just enough to add a method described_class into the context macro into the module that is generated there: https://github.com/waterlink/spec2.cr/blob/master/src/next/dsl.cr#L61

macro context(what, ...)
  # ...

  module {{name.id}}
    # ...

    def described_class
      {{what.id}}
    end

    # ...
  end
end

Both context and describe eventually use this context macro. It might not be enough to define this method like that, because, it might result in compile error, when {{what.id}} is not a proper class or module, but rather just some string. It might make sense to switch on type of AST Node what in the macro:

{% unless what.is_a?(StringLiteral) %}
  def described_class
    {{what.id}}
  end
{% end %}

This still may leave a space for a weird things, like: describe(2+2), which would make described_class return 4. This might be a nice side-effect, but is confusing.

As a workaround for that, maybe change name from described_class to something better?

Another, maybe, more proper, solution, would be to assert, that {{what.id}} is a proper class:

def described_class
  unless {{what.id}}.is_a?(Class)
    raise "#{ {{what.id.stringify}} } is expected to be a class to be used as `described_class`"
  end

  {{what.id}}
end

What do you think? And tell me the results of trying these snippets of code out :)

mauricioabreu commented 8 years ago

I think that is good! I would like to take this moment to thank you for your impressive explanation. It was very good to read it!

If I understood it correct, my pull request #42 does what the feature claims and has the same behavior implemented in rspec.