Crystal: before_each should be scoped within describe block

[georgeu2000]

I'm not sure if this is by design, but this behavior was very surprising to me, and different from RSpec.

describe "Part 1" do
  Spec.before_each do
    puts "before_each"
  end
  it "success" do
    ...
  end
end

describe "Part 2" do
  it "success" do
    ...
  end
end

I expect that the before_each block defined in Part 1 only runs before specs in Part 1. But it seems to run before each spec that is defined after the before_each block, even if they are outside of the Part 1 describe block.

According to the docs,

Instructs the spec runner to execute the given block before each spec, regardless of where this method is invoked.

So, it may be that it is designed to work outside of the describe block. But it does not work as the docs describe either: it only runs on specs defined after the before_each block

[akzhan]

Yes, I'm too frustrated by strange :each blocks.

[georgeu2000]

I took a look at the source, but didn't see how to update it.

[bew]

The *_each methods adds blocks globally (see comment https://github.com/crystal-lang/crystal/issues/4304#issuecomment-294511524), not in a spec scope.

[akzhan]

@bew yes, it's now. But it must be reimplemented in classic setup/teardown way (see https://github.com/crystal-lang/crystal/issues/4304#issuecomment-294520138).

[ysbaddaden]

This is by design.

Duplicate of #4304.

[straight-shoota]

@ysbaddaden Is there an explanation for why this is by design? I honestly can't think of a reason why you would want to have code run before/after every spec in a suite that may have hundreds.

And this is not a duplicate: This issue here is about scoping before/after hooks to certain example groups while #4304 is essentially about execution order that before/after hooks don't run for specs that are lexically defined before them. Please reopen.

[ysbaddaden]

Please search the github issues, there has been explanations about this. I don't use spec myself but minitest, so I won't search further than my memory.

[straight-shoota]

I see no sense in just stating "This is by design" and tagging as a wrong duplicate. This helps no-one to get any wiser.

For reference: There is #1378 which proposed a specific implementation but was discarded due to performance problems. And there is #167 which suggested a number of features for spec but was closed because @asterite said spec works fine.

So it's seems implementing this feature would have a huge impact on compilation time and the recommendation is to explicitly use private methods instead of hooks. Or use an alternative like minitest.

[bew]

We've seen a lot of mis-understanding regarding Spec.*_each methods, what about renaming Spec.before_each to Spec.global_before_each (& after_each => global_after_each) ? This way it'll be more clear that this will register a block to run before/after _all_ following specs, regardless of where (the context) it's called.

[straight-shoota]

... or make it only callable from top level name space. But that's probably not as easy to accomplish.

[robacarp]

The method living at the Module level instead of in the dsl for the describe block is a nice indicator that the logic exists for the global scope and not for the immediate scope. If a local set of _each methods were implemented, would it make sense to simply implement them as part of the spec DSL?

The code might look like this:

    Spec.before_each do
      puts "executed before every test in the suite"
    end

    describe "tests" do
      before_each do
        puts "executed before tests in this describe"
      end

      it "executes both before_each and Spec.before_each" do
        ...
      end
    end

    describe "more tests" do
      it "only executes Spec.before_each" do
        ...
      end
    end

[RX14]

There's no local before_each because it's largely useless to implement them. We have no let (and we shouldn't add it), so they cannot easily pass state to the actual specs. What's the actual usecase which wouldn't be clearer and better implemented using wrapper methods with blocks?

[akzhan]

Anybody should know, that great implementation of RSpec will be greatly appropriated.

And formatters etc.

Simply now core tea have no resources to do all right.

This is strongly IMHO!

[asterite]

If someone wants to implement this, please do so and send a PR. It should support kind of like instance variables, similar to what you can do in rspec or spec2.cr or minitest, otherwise it's pretty pointless.

Good luck 😉

[robacarp]

@RX14 / @asterite these two comments seem in conflict, but perhaps I'm missing some subtlety:

We have no let (and we shouldn't add it), so they cannot easily pass state to the actual specs.

It should support kind of like instance variables, similar to what you can do in ...

Am I missing something about this? I've never been much of an rspec fan, especially the old rspec monkeypatch pattern, so perhaps I'm missing something about what you're saying here because I'm not in the RSpec know.

[RX14]

@robacarp yeah they contradict, the core team is composed of people, naturally we disagree quite often. I take the view that the current spec harness is simple, understandable, and doesn't need any more advanced features. The role of let/before/after can be provided by helper methods in your spec_helper.cr, or private helper methods local to one file. Asterite obviously feels it's better to provide those features in the spec harness itself.

I say they're features which make a single it not self-contained, and dislocate behavior between let/before/after which are spread throughout a file. This makes it really hard to understand, especially with a long file in a large spec suite which you didn't write, you might totally miss a before/after. Helper methods are explicitly called from the it block, meaning they're much harder to hide or get confused about. Go does the same as I described, IIRC.