Spec: detect nesting it/pending and raise compilation error

[makenowjust]

Fixed #7192

Fixed specs following changes also.

require "spec"

it "foo" do
  pending
end

$ crystal run foo.cr
Error in foo.cr:3: instantiating 'it(String)'

it "foo" do
^~

in foo.cr:3: cannot nest 'pending' of spec

it "foo" do
^~

NOTE: Reported location looks wrong due to #7147.

[Sija]

why?

[makenowjust]

Scope.colorize is resolved inside it block before file-scope colorize method, so it needs to change name.

[bcardiff]

Nice tricks! but although I like with expr yield a lot, it might need to go a away in the future in favor of modular/incremental compilation. I'm not 100% sure yet, but I would encourage a runtime check.

[makenowjust]

Indeed. And this check pass over nesting it in some cases.

def nest_test
  it "nest" do
    (1 + 1).should eq(2)
  end
end

it "test" do
  nest_test
end

I'll change this to a runtime check. However, finding problem before running is useful in fact... And, there is no way to detect nesting test case inside pending. Could anyone have another idea?

[asterite]

Let's use a runtime check. with ... yield will embed all tests inside a single file, making compilation halt in some cases, and super slow too.

[straight-shoota]

Specs are typically compiled and executed directly, so there is no big difference between failing at compile time and failing at runtime.

It would probably be possible to implement a compile time check with some macro magic, but that makes it so much more complicated + adds unnecessary compilation overhead.

[makenowjust]

Ok, I decide to rewrite runtime-check this. Thank you @asterite, @straight-shoota, and @bcardiff.

[straight-shoota]

Just a note on the example above: Maybe a single pending with no arguments inside an it block could be allowed to mark the entire sample as pending, like pending {} (instead of it { }) would.
The benefit over the pending block is that it can be placed anywhere in the code which could be a nice feature while developing: You can let the first part of the sample (which is expected to work) execute, but code that comes afterwards (expected to fail - hence pending) is skipped without raising spec errors. The same can be achieved by splitting the sample into two, one it and the other pending. But that's more effort and less flexible.

This is just an idea, maybe it's no effort to implement.

[bcardiff]

@straight-shoota on rspec a pending with block is executed, allowing the developer to code de spec and even check that is not able to run and will fail when the code success (a bit confusing for first timers).

I think that a pending that replaces the it is enough and node/comments in the spec should be enough. I wont expect that asserting things in the middle of an in progress spec will be so much used.