erikdoe / ocmock

Mock objects for Objective-C
http://ocmock.org
Apache License 2.0
2.16k stars 606 forks source link

[FR] Add support for .andFulfill(expectation) #420

Closed dmaclach closed 3 years ago

dmaclach commented 4 years ago

So we have a really nice feature which is

#define andFulfill(anExpectation) _andFulfill(anExpectation)
@property(nonatomic, readonly) OCMStubRecorder * (^_andFulfill)(XCTestExpectation *);

it allows you to write

OCMStub([foo bar]).andFulfill([self expectationWithDescription:@"Hit bar"]);
...
[self waitForExpectationsWithTimeout:0 nil];

as opposed to

XCTestExpectation *expectation = [self expectationWithDescription:@"Hit bar"];
OCMStub([foo bar]).andDo(^(NSInvocation *invocation) {
    [expectation fulfill];
  };
...
[self waitForExpectationsWithTimeout:0 nil];

Which is really nice when you are writing a bunch of mocks with expectations.

Are we okay with adding it? The major side effect is that it means that OCMock will have a dependency on XCTest.

erikdoe commented 4 years ago

Can you think of any downside in the real world of depending on XCTest? Are there codebases that do unit testing without XCTest? Is there even an alternative left?

dmaclach commented 4 years ago

I don't have any reason to avoid XCTest, I just wanted to make sure that it was recognized that we were taking on the dependency. I am unaware of any alternatives.

imhuntingwabbits commented 4 years ago

Clients targeting releases older than iOS 7 (XCTest was introduced in Xcode 5) won't have xctest.

That said XCTestExpectation was new in Xcode 7.2 for iOS 9 aligned SDKs. That may pose some risk here. Should these be wrapped by / annotated with availability macros?

dmaclach commented 4 years ago

Isn't it the SDK that they are building with which is key? AFAIK Apple isn't accepting anything built with less than iOS 13 SDK starting in July.

On Sat, May 23, 2020 at 4:14 PM imhuntingwabbits notifications@github.com wrote:

Clients targeting releases older than iOS 7 (XCTest was introduced in Xcode 5) won't have xctest.

That said XCTestExpectation was new in Xcode 7.2 for iOS 9 aligned SDKs. That may pose some risk here. Should these be wrapped by / annotated with availability macros?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/erikdoe/ocmock/issues/420#issuecomment-633152469, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACOFSJBGSVR4K2Z6E33Z63RTBKELANCNFSM4NHGKSUA .

imhuntingwabbits commented 4 years ago

Isn't it the SDK that they are building with which is key?

The Deployment Target is used to limit API availability.

I'm not hard set on any of this, my personal preference is target what's available and as far as I know you can't set a pre-iOS 9 deployment target in any modern Xcode?

So... mute point? Clients of OCMock with "special" configurations can push PRs when they discover this?

dmaclach commented 4 years ago

Yeah, in this case since it's a developer tool (and not a OS API) there is no deployment target flags. So either you have XCTest in your Xcode or you don't. I think we are good.

sigito commented 4 years ago

Just another option: fulfillment can be linked to the method execution with .andCall(expectation, @selector(fulfill)) instead of a block dance with .andDo(...).

dmaclach commented 4 years ago

I like that better. Updated.

dmaclach commented 4 years ago

FYI turns out that andCall is not correct in that it changes the return value and something like .andReturn(YES).andFullfill(expectation) ended up returning NO. Reverted back to the andDo version

erikdoe commented 3 years ago

Merged the corresponding PR.