Giter Site home page Giter Site logo

Comments (22)

AliSoftware avatar AliSoftware commented on May 21, 2024

Hi,

Do you have a small/minimalist example project where you can reproduce this issue?

As the assertion states, this seems illogical that startLoading is called if no stub has been identified to respond to the request beforehand, but maybe I missed a tricky case?

Something related to multihreaded & race-conditions maybe? (like canInitRequest being called at a time there are some stubs installed, but the stub being removed in between before startLoading is called, even if all of this should happen in the same runloop? But I can't see any use case where this would happen…)

Anyway, an example project would help me a lot to debug this!

from ohhttpstubs.

AliSoftware avatar AliSoftware commented on May 21, 2024

In addition, did you try to removeAllStubs in tearDown instead of setup?
It seems to me more logical to clean up the stubs you installed in a test case after this test case (instead of at the beginning of the next one) and it maybe solve your crash (even if I prefer to find a proper solution to fix this issue correctly so it can never happen again)

from ohhttpstubs.

marcelofabri avatar marcelofabri commented on May 21, 2024

I'll try to do it in tearDown. However, if a test fails with an exception, for example, tearDown is not executed, making my other tests run on a "dirty" environment (please correct me if I'm wrong). I'll try to make a sample project later today.

I'm using Expecta on my project too, to test async behavior.

from ohhttpstubs.

AliSoftware avatar AliSoftware commented on May 21, 2024

Are you sure about that? Normally every Test framework correctly built (XCTest, OCUnit, GHUnit, etc…) execute its tearDown method even when the test fail or generates an exception (which is catched by the test framework, to mark the test as failed — except if you XCTAssertThrow and did expect the exception to occur of course in which case it marks the test as succeeded).

That's what makes tearDown so interesting actually.

from ohhttpstubs.

marcelofabri avatar marcelofabri commented on May 21, 2024

I made a small test with XCTest (running tests inside Xcode) and that was the conclusion I figured out.
I created a XCTestCase like this:

@interface ExceptionTests : XCTestCase

@end

@implementation ExceptionTests

- (void)setUp {
    [super setUp];
    // Put setup code here. This method is called before the invocation of each test method in the class.

}

- (void)tearDown {
    // Put teardown code here. This method is called after the invocation of each test method in the class.
    [super tearDown];
    NSLog(@"TEAR DOWN is called!");
}

- (void)testThrowException {
    @throw [NSException exceptionWithName:@"TestException" reason:@"Test" userInfo:nil];
}

@end

If I comment @throw, my log is printed, but if I let it the log is never printed (and tearDown is not called - I put I breakpoint on it too).

I'm new to XCTest (and unit tests in general), so maybe I'm missing something.

from ohhttpstubs.

dreed1 avatar dreed1 commented on May 21, 2024

i just wanted to chime in that I'm having the same exact problem, although I'm not trying to remove any stubs anywhere but my test suite teardown methods.

I don't have time right now to write a reproducible project, but I'll try to make some in the next few days.

I think you're right, that there's a race condition somewhere.

I've got 4 test suites that all create their own stubs in the setup block, and remove them in the teardown. The last one gives me this null stub error.

Something like

@interface someTestSuite : XCTestCase {
  id httpStub;
}

- (void)setUp
{
  [super setUp];

  [OHHTTPStubs setEnabled:YES];
  httpStub = [OHHTTPStubs stubRequestsPassingTest:^BOOL(NSURLRequest *request) {
    return YES;
  } withStubResponse:^OHHTTPStubsResponse*(NSURLRequest *request) {
    NSString *stubPath = PATH_TO_A_JSON_FIXTURE;
    if (stubPath != nil) {
      return [OHHTTPStubsResponse responseWithFileAtPath:stubPath
                                              statusCode:200
                                                 headers:@{@"Content-Type":@"text/json"}];
    }
    return [OHHTTPStubsResponse responseWithData:nil
                                      statusCode:200
                                         headers:@{}];
  }];
}

- (void)tearDown
{
  [OHHTTPStubs removeStub:httpStub];
  [super tearDown];
}

and the stub fails that same assert: 'Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'At the time startLoading is called, canInitRequest should have assured that stub is != nil beforehand''. My stacktrace is exactly the same as above, except line 5 is my code instead of his.

Hopefully this helps a little... I'll see if I can get you something reproducible within a few days. I'm out of sprint time for bug resolutions for the week so I probably can't get to it until the weekend starts. Thanks for all the hard work, this is a great little library.

from ohhttpstubs.

AliSoftware avatar AliSoftware commented on May 21, 2024

@dreed1 Thanks for the feedback.

I probably have an idea of the cause: do both of you ensure that the request has time to receive a response — or to timeout — before the end of your test cases?

Especially, NSURLConnection (and any other way to send an NSURLRequest) are generally asynchronous (in a dedicated NSURLConnection thread), so if you don't wait for some time and don't block your test case code to let the request being sent (and catched by OHHTTPStubs) and instead let your test code finish before the request had time to be sent/processed, then the test code will end… and the tearDown will be called (thus removing your stub) before the request was finished processing.

You should at least use something like [[NSRunLoop mainRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:1]] for example, to let the runloop run some iterations and the request to be processed (see -(void)waitForAsyncOperations:(NSUInteger)count withTimeout:(NSTimeInterval)timeout for an example on how I do this in my own code)

from ohhttpstubs.

marcelofabri avatar marcelofabri commented on May 21, 2024

I'm using Expecta's will matcher, with a timeout of 5s.

from ohhttpstubs.

AliSoftware avatar AliSoftware commented on May 21, 2024

Damn so that's not the cause…

Still interested by some code to reproduce it, understand what exactly you are doing and what can cause this…

from ohhttpstubs.

marcelofabri avatar marcelofabri commented on May 21, 2024

I think you were right about your idea: I have tests where I don't use will operator, because I'm not interested in the response - just on whether the request was built successfully.

However, I wasn't able to isolate it to a sample example. Even on my regular project it's hard to reproduce the issue (I'd say less than 1% of times running the full test suite).

PS: I was also able to reproduce the issue when removing the stubs on tearDown instead of setUp.

from ohhttpstubs.

AliSoftware avatar AliSoftware commented on May 21, 2024

Ok thanks for the feedback @marcelofabri

So I'll start on the assumption that this only happens when you remove the stub after you sent the request but before waiting for the response to be returned (or the timeout to be exceeded), typically when you don't block your Test Case to wait for it (like when you forget to use Expecta's will operator)

I'll try to secure all this a bit more anyway to avoid a crash in such case. Maybe just removing the assertion (and just returning directly without doing anything in startLoading when the stub is nil — even if it wasn't by the time canInitRequest: was hit) will suffice… but I think issuing some warning to let the user know that something was odd/wrong was a nice idea…

If you have any suggestion on how I could detect and warn the user properly so that s/he realizes that s/he may have forgotten some active wait method (like the will operator) in one of his/her test case or something… and to make sure their test results are deterministic and not with a random race condition in such case… that would be nice :)

from ohhttpstubs.

duanefields avatar duanefields commented on May 21, 2024

I see the same symptoms, but I see it maybe 25% of the time, which smells like a race condition, when running "all tests".

from ohhttpstubs.

duanefields avatar duanefields commented on May 21, 2024

@AliSoftware that turned out to be exactly the problem - I tear down the stub after each test, but some of the tests weren't waiting for the response to complete (because I didn't care about the result) which caused the race condition

from ohhttpstubs.

gfontenot avatar gfontenot commented on May 21, 2024

Just want to chime in here and say that I'm seeing the exact same behavior. Using Specta/Expecta, the will matcher, NSURLSession, and clearing all stubs on tearDown via Specta's afterEach block.

from ohhttpstubs.

AliSoftware avatar AliSoftware commented on May 21, 2024

Ok, so I should probably change the message of the assert so that it is a bit more explanatory, like:

You started a request that was stubbed by OHHTTPStubs, but removed the stub before the request even had time to finish. If you are using OHHTTPStubs in your Unit Tests, be sure to block your test cases until the request has finished and the response has arrived, so that the test doesn't finish and the stub doesn't get removed before the response was complete!

I should even definitely write a wiki article at some point to explain this use case more thoroughly and how you should make your test cases wait for the response to arrive before the test can end and its status (success/failure) can then correctly determined. I would then put a link to this article in the assertion message to help users that encounter this.
Maybe even don't make it an assertion, but just a log message to help users understand that asynchronous tests always have to somehow block the test case at the end before all asynchronous operations are finished.


Will hopefully do that this weekend, but please remind me to do so if I didn't come up with this article past next week, I got a lot on my plate and I am able to forget about it 😉

from ohhttpstubs.

gfontenot avatar gfontenot commented on May 21, 2024

fwiw, I had 0 synchronous expectations in my suite. Everything was using will, nothing used to. I was getting these issues returning a super simple bit of JSON data with a (default) 1 second timeout.

from ohhttpstubs.

AliSoftware avatar AliSoftware commented on May 21, 2024

I never used Expecta, but shouldn't will precisely wait / block the code/current thread, until the expectation is reached (or timeout)? Then if that's the case and if you use will in every of your tests, I don't understand why you would have the assertion, as will would wait for the request/stub to be finished and the response to be fully arrived to unblock the code and continue, right?

Maybe you could c/p a sample test of your Test Suite here to help me understand why you would hit the assertion in your specific case?

from ohhttpstubs.

gfontenot avatar gfontenot commented on May 21, 2024

You're exactly right about the expected behavior there. I couldn't figure out what the hell was going on either. Let me see if I can come up with a super simple reproduction when I get home, and I'll throw it up somewhere that you can get to.

from ohhttpstubs.

gfontenot avatar gfontenot commented on May 21, 2024

@AliSoftware Here's a test project I zipped up showing the exception.

It happens really intermittently, you may have to run it 10 or more times. After pruning it down to the essentials, it hit the exception on the second run, but then I had to run it ~12 times to hit the next assertion.

from ohhttpstubs.

AliSoftware avatar AliSoftware commented on May 21, 2024

This last commit should fix it, by storing the stub in a @property as soon as possible to be sure to keep it around (and by replacing the NSAssert call with a simple NSError in case this race condition should even happen again)

➡️ Version 3.1.1 has been pushed to CocoaPods with this fix.


@gfontenot It retried your example project after applying this fix and wasn't able to reproduce the issue even after running your Unit Tests about 20 times.
But as this happens intermittently I may have been very lucky and just never got into the bad race condition since, so don't hesitate to keep me posted if you happen to fall into this issue again.

from ohhttpstubs.

AliSoftware avatar AliSoftware commented on May 21, 2024

ping @gfontenot Is this still the issue you are dealing with on Twitter? Did you try to pod update OHHTTPStubs on your project?

from ohhttpstubs.

gfontenot avatar gfontenot commented on May 21, 2024

Nope, the more recent versions seem to have solved this particular issue.

The issues I'm talking about on twitter seem to be completely unrelated to OHTTPStubs. Pretty much grasping at straws for those.

from ohhttpstubs.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.