calabash / calabash-ios-server

An embedded HTTP server for performing queries and test automation
Other
87 stars 82 forks source link

Enable tests to dispatch blocks and wait for their results on the main dispatch queue #393

Closed Lievesley closed 7 years ago

Lievesley commented 7 years ago

Use performSelectorOnMainThread instead of dispatch_sync because using a dispatch method prevents further blocks from being processed on the main queue until the initial block in which the httpResponseOnMainThreadForMethod was invoked has completed. dispatch_sync prevents other blocks being processed in nested calls to NSRunLoop run methods within tests.

calabash-ci commented 7 years ago

Can one of the admins verify this patch?

msftclas commented 7 years ago

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. We will now review your pull request. Thanks, Microsoft Pull Request Bot

jmoody commented 7 years ago

Jenkins test this please

jmoody commented 7 years ago

@Lievesley Can you provide an example of what problem this change solves?

Lievesley commented 7 years ago

@jmoody Here is a simplified code example of a custom Calabash action that is currently broken as no blocks are processed on the main queue until the initial dispatch_sync block completes.

__block BOOL flag = NO;
dispatch_after(dispatch_time(DISPATCH_TIME_NOW, (int64_t)(1 * NSEC_PER_SEC)), dispatch_get_main_queue(), ^{
    flag = YES;    // Without the fix in this PR this line only gets called after the initial dispatch_sync completes
});
NSDate* loopUntil = [NSDate dateWithTimeIntervalSinceNow:10];
while ([loopUntil timeIntervalSinceNow] > 0 && !flag)
{
    [[NSRunLoop mainRunLoop] runMode:NSDefaultRunLoopMode beforeDate:loopUntil];  // Does not process blocks on main queue as main queue is blocked waiting for dispatch_sync to complete
}
NSAssert(flag == YES, @"Flag was not set because main queue was blocked");
jmoody commented 7 years ago

this line only gets called after the initial dispatch_sync completes

I think this is the behavior we want. Can you describe how this is blocking a test?

Lievesley commented 7 years ago

I think this is the behavior we want. Can you describe how this is blocking a test?

It's not the behaviour that we want since it means that calling: [[NSRunLoop mainRunLoop] runMode:NSDefaultRunLoopMode beforeDate:loopUntil]; does nothing useful. What we want to do is to process the main queue in this runMode method.

In our test scenarios we have backdoor methods that call asynchronous operations that dispatch events back to the main queue. We wait for these events to be processed on the main queue using: [[NSRunLoop mainRunLoop] runMode:NSDefaultRunLoopMode beforeDate:loopUntil]; The asynchronous events in question must complete before the backdoor method returns. This cannot happen if the main queue is blocked by the dispatch_sync call from the Calabash framework.

Using performSelectorOnMain thread enables the scenario I have just described while not breaking any other test case. If you disagree please provide an example of why it is useful for: [[NSRunLoop mainRunLoop] runMode:NSDefaultRunLoopMode beforeDate:loopUntil]; not to work inside the backdoor method.

jmoody commented 7 years ago

Thank you for the explanation.

jmoody commented 7 years ago

@Lievesley If you make me (@jmoody) a collaborator on your fork, I will add the backdoor test to the LPTestTarget app.

Lievesley commented 7 years ago

Thanks! I just added you as a collaborator.

jmoody commented 7 years ago

Jenkins test this please

jmoody commented 7 years ago

Tests are failing on Jenkins because of a problem with develop. Will resolve the issue in a subsequent commit on origin/develop.