Ensure let blocks are evaluated only once per example #493
Conversation
…declaring parameter list void
|
I just want to say that this solves the problem I've got when I tried to use @sharplet thanks a lot! I've upgraded to Kiwi 2.2.4 and have tried to rewrite some tests to use Here is how I'm using describe(@"Foo", ^{
__block NSDate *date;
let(bars, ^id{
// some code which uses `date` variable and returns NSArray
return @[]; // some code which returns NSArray
});
context(@"when AAA", ^{
it(@"returns aaa", ^{
date = [NSDate mt_dateFromYear:2014 month:6 day:10];
[[bars should] haveCountOf:1];
[[bars should] containObjects:obj1, nil];
});
});
context(@"when BBB", ^{
it(@"returns bbb", ^{
date = [NSDate mt_dateFromYear:2014 month:1 day:10];
[[bars should] haveAtLeast:3];
[[bars should] containObjects:obj2, nil];
});
});
});Looking forward to see this in master and to have a new release :) |
|
Also a short question: when I add a breakpoint inside a block passed to
|
|
@yas375 I believe this may just be the way it works with macros: see @dmeehan1968's comment here. |
|
@sharplet thank you very much for your work here. I think we should merge this to fix the existing The problem occurs if someone already includes them in the codebase, it won't compile anymore. |
|
@supermarin No problem. A couple of strategies I've thought about for MAFutures:
I'm leaning towards 2 at the moment (or something close to it), on the grounds that contributing what we can back to the community is a good thing. |
|
/ping @sharplet I'm thinking of releasing 2.3. |
Conflicts: Kiwi.podspec Kiwi.xcodeproj/project.pbxproj Tests/KWFunctionalTests.m
|
Yo. I think we should merge it. I was considering merging MAFuture as a subtree of Kiwi, but decided I didn't really want to have all of MAFuture's history as part of Kiwi (for now). I'm a little concerned about the compatibility issues, however. I almost think it would be better if they were likely to cause hard compiler errors, as that would be easier to debug. However because we'll be compiled in the user's test bundle and then injected into their app during testing, any possible issues are more likely to manifest as subtle runtime bugs. Because the code is just copied in and shipped with Kiwi, we're really responsible for it. So I think I've convinced myself that we should re-namespace the code so we can be more certain about what will happen at runtime. Do you agree? |
This basically treats MAFuture as an internal implementation detail of Kiwi. If an app using Kiwi for testing itself includes MAFuture, there likely won't be duplicate symbol errors at compile time as Kiwi will be compiled into the test bundle, and then injected at runtime. Prefixing MAFuture symbols with KW_ mitigates the risk of undefined behaviour in this scenario.
|
Sounds good to me, since this is only the test target. @modocache final thoughts? we're close to 🚢 !!! 🍻 |
|
@supermarin Thanks for the ping!
I think that's a very wise decision. Personally, this pull request is a bit too complex for my tastes, however. I'm not looking forward to maintaining the futures code. I'm also of the opinion that But considering |
|
@modocache thanks for the input! My thoughts were the same - there must be a way to deal with it in a simpler way, I just haven't had time to dig too deep in this code anymore :( |
|
@supermarin @modocache Definitely not negative, I agree this has gotten much more complicated. Pretty sure if I'd originally submitted a PR this heavy you would have politely declined ;) Thanks for the feedback! |
Conflicts: Kiwi.xcodeproj/project.pbxproj
…ectations
When futures are involved, it seems that -isEqual: is not necessarily
commutative. For example, these two aren't guaranteed to be equivalent:
[[futureObject should] equal:actualObject];
[[actualObject should] equal:futureObject];
In the case of string comparison, it is the second form that fails. An
MAFuture is very lightweight proxy for the resolved future value --
almost all messages the future receives are forwarded to the real object
it represents. However when a future string is on the right-hand side of
-isEqual:, the comparison fails. My best guess is that the
implementation of -isEqualToString: doesn't send any messages to the
right operand, and accesses its internal storage directly.
|
@modocache @supermarin I've just added a failing test for an issue I discovered when using future strings in expectations, see the build results here: https://travis-ci.org/kiwi-bdd/Kiwi/builds/29403601. I also thought of a potential fix and pushed it here: https://github.com/sharplet/Kiwi/commit/b57b4de786e1c5380df62dfabc8716c1616c455d. I'm a little torn. The fix works great, and even though it does introduce a performance penalty in the negative case, I feel like the logic is valid. Still, I can't quite fight the feeling that maybe it's too much of a special case to fix it with such a sledgehammer—and of course the underlying problem still exists. Of course, if you have any ideas for an alternate fix I'm all ears! 😉 |
|
This looks like an interesting experiment, and I've just discussed this with @sharplet and @modocache and the complexity it adds probably isn't worth it. So, as it's been inactive for over two years, I'm closing this. As ever - this PR can be re-opened if the idea is worth bringing back up, but it looks like discussion has petered out. |
This resolves #433.
Instead of holding a direct reference to its block,
KWLetNodenow turns it into a future (using https://github.com/mikeash/MAFuture).When a node is evaluated, its pointer and all its children's pointers are assigned a reference to the future. The important difference here is that the block still hasn't been evaluated. Because it's now a future, the block isn't evaluated until the first time the future object is messaged, at which point the block is invoked and the message send is proxied to the real object.
That is, we now have real lazy evaluation, just like RSpec! Yay! 🎉
The let node tree is now unlinked, and the values reset, at the end of each example (rather than after the last example in a context, which was incorrect).
I'm still not 100% certain that this was the only solution. But figuring out the correct order in which to evaluate let nodes, in order to "simulate" lazy evaluation, was a big headache. Introducing futures made this problem go away by decoupling let node tree evaluation (assigning object references to let helper variables) from actual let block evaluation. So when the tree is evaluated we can just
without worrying about side effects. And then when a future is actually used the block is lazily evaluated as needed.
Hope that makes sense! 😅
And here's the functional test that verifies the behaviour: https://github.com/sharplet/Kiwi/blob/let-futures/Tests/KWFunctionalTests.m#L207-226.
If everyone's happy to go ahead with this, the question of the best way to import a third-party dependency still remains. I've just copied the source code from MAFuture
HEADinto the tree for now, for testing. For CocoaPods, I've exported the code as a subspec. Would a git submodule be better for bringing in the source? Additionally, it's also bringing in a significant chunk of code that's still MRC, which doesn't make me super happy given the awesome progress recently on converting to ARC!