let blocks in nested contexts are executed multiple times

[MichaelHackett]

I'm trying out the new "let" feature, and I found that a mock I partly configured in an outer context was losing that configuration when used in an inner context. Digging into the code, it seems that it's build a "let tree", combining all the ancestor let notes with those at the current level. But the performExample:withBlock: code in KWContextNode already builds a tree of contexts to process before executing an it block (so that outer beforeEach and afterEach blocks still get executed before every inner it), so the outer let nodes are getting executed twice.

Demo:

SPEC_BEGIN(KiwiLetBugDemoSpec)

describe(@"Outer context", ^{
  let(foo, ^{ NSLog(@"outer let executed"); return (id)nil; });
  beforeEach(^{ NSLog(@"outer beforeEach executed"); });

  context(@"Inner context", ^{
    let(bar, ^{ NSLog(@"inner let executed"); return (id)nil; });
    beforeEach(^{ NSLog(@"inner beforeEach executed"); });

    it(@"should do nothing", ^{});
  });
});

SPEC_END

Output:

Test Case '-[KiwiLetBugDemoSpec OuterContext_InnerContext_ShouldDoNothing]' started.
2014-01-19 16:03:48.600 otest[22290:7803] outer let executed
2014-01-19 16:03:48.601 otest[22290:7803] outer beforeEach executed
2014-01-19 16:03:48.601 otest[22290:7803] outer let executed
2014-01-19 16:03:48.602 otest[22290:7803] inner let executed
2014-01-19 16:03:48.602 otest[22290:7803] inner beforeEach executed
2014-01-19 16:03:48.603 otest[22290:7803] + 'Outer context, Inner context, should do nothing' [PASSED]
Test Case '-[KiwiLetBugDemoSpec OuterContext_InnerContext_ShouldDoNothing]' passed (0.004 seconds).

I don't think this was the intended behavior --- I would have expected it to work the same as beforeEach --- but I wanted to check before making any attempt to change it. At best, it's doing unnecessary work, recreating the value multiple times per example, but in the case I ran into, it actually broke the test.

[sharplet]

Thanks for the report. I'm planning to take a look into this; you're right that the behaviour doesn't seem quite correct. I have a feeling that we may need to somehow mark a let node as being executed in a given context. I don't think it's quite as simple as that though: for example, what if foo was redefined in the inner context? In that case it would be necessary to reevaluate bar, as it's value may depend on foo.

So it's worth looking into, however I'm not 100% certain yet that the behaviour is necessarily incorrect.

Would it be possible for you to post a spec that shows exactly what you mean by the mock losing configuration in an inner context?

[MichaelHackett]

Ah, I see. I was wondering why it didn't just work like beforeEach, but it's because of the redefinition issue. Outer lets may need to be re-executed given in the new context. Tricky...

Unfortunately, I seem to have deleted the exact spec that initially caused me to look into this, but I've put something else together (below). From doing this exercise, I think there's only an issue if one mixes let and beforeEach, because the latter are not re-executed when the let variables are redefined. So, in this example, the problem would be avoided if I did the setFoo: call within the let block. But if the change only applied to some examples, it has to be done in an inner beforeEach, unless you repeat the whole let. (Hope that makes sense.)

Besides effectively allowing multiple beforeEach blocks and a slightly clearer (though not necessarily more terse) syntax for declaring variables, is there some other benefit to let? Just allowing multiple before/afterEach/All blocks would make it easier to write custom setup macros (to improve test readability), and might be a simpler change to integrate with the existing structure.

#import <Kiwi/Kiwi.h>

@protocol Foo <NSObject>
- (NSString*)status;
@end

@interface ExampleClass : NSObject
@property (strong, nonatomic) id<Foo> foo;
@end
@implementation ExampleClass @end


SPEC_BEGIN(KiwiLetBugDemoSpec)

describe(@"Example class with beforeEach", ^{
  __block id subject;
  __block id testFoo;
  beforeEach(^{
    testFoo = [KWMock mockForProtocol:@protocol(Foo)];
    subject = [ExampleClass new];
    [subject setFoo:testFoo];
  });

  context(@"status method", ^{
    beforeEach(^{
      [testFoo stub:@selector(status) andReturn:@"bar"];
    });

    it(@"should receive message", ^{
      [[[[subject foo] status] should] equal:@"bar"]; // passes
    });
  });
});

describe(@"Example class with let", ^{
  let(testFoo, ^{ return [KWMock mockForProtocol:@protocol(Foo)]; });
  let(subject, ^{ return [ExampleClass new]; });
  beforeEach(^{
    [subject setFoo:testFoo];
  });

  context(@"status method", ^{
    beforeEach(^{
      // this is not the same testFoo that was assigned to the subject above
      [testFoo stub:@selector(status) andReturn:@"bar"];
    });

    it(@"should receive message", ^{
      [[[[subject foo] status] should] equal:@"bar"]; // fails
    });
  });
});

SPEC_END

[y310]

I encountered same issue.
This is example code to reproduce this problem.

SPEC_BEGIN(FooSpec)

describe(@"foo", ^{
    let(foo, ^id{ return @"foo"; });

    beforeEach(^{
        NSLog(@"outside before: %@", foo);
    });

    context(@"bar", ^{
        let(foo, ^id{ return @"foobar"; });

        beforeEach(^{
            NSLog(@"inside before: %@", foo);
        });

        it(@"should work", ^{
            NSLog(@"it: %@", foo);
        });
    });
});

SPEC_END

This code prints following output

outside before: foo
inside before: foobar
it: foobar

I expect to get foobar in outside beforeEach but inside let is evaluated after outside beforeEach.

I ran same kind of spec by rspec and it works as I expected.

describe "foo" do
  let(:foo) { "foo" }

  before do
    puts "outside before: #{foo}"
  end

  context "bar" do
    let(:foo) { "foobar" }

    before do
      puts "inside before: #{foo}"
    end

    it "should work" do
      puts "it: #{foo}"
    end
  end
end

output

outside before: foobar
inside before: foobar
it: foobar

rspec behavior is "overwrite all lets by most inside of let" but kiwi does not work like it.

[cristiankocza-sv]

@y310 what you noticed is a side effect of the fact that you only have one test, declared in the inner context. Basically when rspec evaluates the inner "it" it also evaluates all "let" and "before" declaration, "let" having priority here. So in your example the outer "before" is executed in the context of the inner test, thus the inner "let" is the one that decides the value of "foo". Below is a modified version of your tests, if you run them you'll see that the outer "let" is correctly used for the outer tests:

describe "foo" do
  let(:foo) { "foo" }

  before do
    puts "outside before: #{foo}"
  end

  it "outside test" do
    puts "outside test: #{foo}"
  end

  context "bar" do
    let(:foo) { "foobar" }

    before do
      puts "inside before: #{foo}"
    end

    it "should work" do
      puts "it: #{foo}"
    end
  end
end

Output

outside before: foo
outside test: foo
.outside before: foobar
inside before: foobar
it: foobar
.

Finished in 0.00104 seconds
2 examples, 0 failures

[jonasfa]

Just lost an hour of work because of this.
It's disappointing to see it's been around for half a year and wasn't fixed yet.

[dueckes]

+1 on replicating RSpec's behavior here.
Would be great to 'redefine' a let to provide a different value for a behavior.

[bilby91]

Any progress on this ?