Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make it possible to pass RACSignalAsynchronousWaitTimeout #95

Closed
jcavar opened this issue Apr 27, 2017 · 4 comments · Fixed by #98
Closed

Make it possible to pass RACSignalAsynchronousWaitTimeout #95

jcavar opened this issue Apr 27, 2017 · 4 comments · Fixed by #98

Comments

@jcavar
Copy link
Contributor

jcavar commented Apr 27, 2017

Hi,

RACSignalAsynchronousWaitTimeout is used in - (id)asynchronousFirstOrDefault:(id)defaultValue success:(BOOL *)success error:(NSError **)error but it is not possible to change that value. If test is expected to take longer then defined value, it will fail.

Would it be possible to create alternative method that accepts this value as well?

@erichoracek
Copy link
Contributor

I've found in the past that when I'm bumping up against the time limits of the asynchronous... family of methods in tests, the solution is typically to attempt to speed up the test rather than increase the duration of the wait.

With respect to adding a new method, I'd learn towards actively discouraging long-running test methods in ReactiveObjC unless there's a very good use case for allowing custom wait durations.

However, if you absolutely need a longer timeout, it should be relatively easy to roll your own asynchronous... wait method as a RACSignal extension.

@jcavar
Copy link
Contributor Author

jcavar commented Apr 28, 2017

We are using it for performance testing with quite large data set, so we expect it to be longer then 10, especially on our Jenkins machines that are relatively slow.

But regardless of that I think allowing to pass dependency is generally good idea. Someone maybe needs shorter time interval. And we can still keep default value 10.

@erichoracek
Copy link
Contributor

That use case seems like something that could be supported by ReactiveObjC if you'd want to take a stab at it. We'd likely want to keep the original asynchronous... methods but add another set that takes an extra timeout:(NSTimeInterval) for backwards compatibility.

@jcavar
Copy link
Contributor Author

jcavar commented May 4, 2017

Yes, that makes sense. Cool, I will then work on this when I have some free time and will post PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants