Comments (14)
Hi Vladimir, if I'm not mistaken, you're concerned about a test case (presumably, integration one?) where the user may create a bunch of promises internally, which may not be related directly to the functionality being tested, and thus create a race condition when waitForPromises
is being used for synchronization, which eventually would result in a flaky test?
My take on that is the user should be well aware of what waitForPromises
does, and gladly use XCTest framework when the former appears insufficient or potentially error-prone for a specific test case. Luckily, waitForPromises
is not the only true way to test promises, it's just the simplest one.
Thanks.
from promises.
@shoumikhin Yes, I'm concerned about that test case which will be quite common for any complex asynchronous system (and async code is complex by default). Also I'm concerned that that method will be used in production code of that system (for example, in SDK), which will create deadlock if I will use your Promises library in my app that uses SDK that uses your library too.
waitForPromises is not the only true way to test promises, it's just the simplest one
And also most incorrect one, so everyone will be accustomed to incorrect way from the start.
You cannot determine that all promises are completed anyway (so method name is a lie) due to fundamental Halting Problem: just after waitForPromises() return, some background thread/queue can just create yet another promise.
from promises.
Not sure I get why you consider it incorrect? Any examples?
Also, why do you think that method is gonna be used in production? It's located in a separate header in Objective-C, and is internal in Swift, so you can't just leverage it for anything w/o explicitly importing a test-only declaration.
I totally get that another promise can be created after waitForPromises
and thus, stay unfulfilled. Then your next test will likely time out, so you're gonna investigate what went wrong and probably find a bug or fix the test case to become isolated.
from promises.
Not sure I get why you consider it incorrect? Any examples?
Yes, consider that iOS would have following method of NSThread class for async testing:
- (void) waitForAllThreadsExceptMain()
And that Apple would recommend its usage in XCTest documentation as simplest way to write asynchronous test.
You cannot wait for ALL threads and promises, even in tests, because threads and promises are ALWAYS implementation detail that may not be visible to the test and that are thus prone to race conditions from test's point of view.
I totally get that another promise can be created after waitForPromises and thus, stay unfulfilled.
Or tested code can create arbitrary number of promises while you're waiting in waitForPromises(). And it would do it as part of its CORRECT operation.
That's the core problem: you cannot distinguish freeze of the system from its correct operation. That was proved by Alan Turing in 1936: https://en.wikipedia.org/wiki/Halting_problem
Your method waitForPromises() incorrectly implies directly opposite to that.
Then your next test will likely time out, so you're gonna investigate what went wrong and probably find a bug or fix the test case to become isolated.
No, you would not find that bug, because it will be the result of asynchronous race condition by definition.
Or your waitForPromises() would just hang sometimes for arbitrary amount of time for CORRECT tests with CORRECT code.
from promises.
Sorry, if I understand you correctly, your concern around waitForPromises
is that it can be potentially misused and become error-prone if blindly used everywhere?
Do you suggest to refine the docs then, and explicitly recommend using XCTest instead if unsure or facing issues with waitForPromises
?
from promises.
if I understand you correctly, your concern around waitForPromises is that it can be potentially misused and become error-prone if blindly used everywhere?
No, my concern is that any use of waitForPromises() is inherently incorrect, because it implies wrong assumptions that:
- Tested code do not use promises in its internal implementation — and thus makes async test to be dependent on implementation details and fail when tested code still works correctly but just changed implementation.
- It can distinguish between freezed and correct states of tested code when it creates large number of promises in its internal implementation.
In overall, this method represents completely incorrect way of writing async tests: instead of waiting for the overall state of the async system (which is prone to deadlocks and race conditions) you must wait for particular events from it.
XCTestExpectation does exactly that.
Do you suggest to refine the docs then, and explicitly recommend using XCTest instead if unsure or facing issues with waitForPromises?
No, I suggest removing waitForPromises() entirely, because its testing paradigm is entirely incorrect and also useless on platform that have perfect async code testing capabilities like XCTestExpectation.
And current waitForPromises() implementation is awful anyway because it works like spinlock and eats CPU for breakfast, which will lead to excessive CI server usage. So nothing will be lost from its removal anyway.
from promises.
Sure, so you may perceive waitForPromises
as a mean to wait for an event "all pending promises have been resolved up to this moment". That's the only assumption it implies. If you want to wait for some other system state, and are able to expose it in tests for proper event handling, you're welcome to use XCTest framework exclusively and never touch that utility method.
from promises.
waitForPromises as a mean to wait for an event "all pending promises have been resolved up to this moment". That's the only assumption it implies.
It is not an event, it is implementation detail of the tested code, which may change without changing code's correct external behaviour. Therefore, waitForPromise() should not be used in tests, because it makes tests dependable on hidden implementation details of tested code.
from promises.
I think it really depends on how you look at it and what exactly you test.
Sounds like you're talking about some integration tests and check for a specific set of events to happen, while ignoring the others. And you also don't expect a test to change much while working on implementation details. Surely, waitForPromises
may not be an ideal candidate for that.
from promises.
Sounds like you're talking about some integration tests and check for a specific set of events to happen, while ignoring the others.
No, I'm talking about the fact that test should not depend on tested code's implementation details that it cannot see.
Your promises can (and will) be used in implementation of classes that return OTHER promises via external public API. waitForPromises() will immediately lead to incorrect behaviour of tests in that case. Because it uses fundamentally wrong testing paradigm.
from promises.
I see. Well, waitForPromises
isn't considered a panacea or something every test that rely on promises must use. It's mostly used to test the lib itself. Nevertheless, we found a few other good use cases where we couldn't explicitly expose all the events to the test methods in order to observe completion, but had to wait for them somehow. Hope that helps. Feel free to send a PR to refine the docs, if needed.
from promises.
My position is that it should be removed from library, not have changed just changed docs. So no PR. :)
Exposing internal events to test can be done via Swift’s internal keyword and following import:
’’’
@testable import MyFramework
’’’
from promises.
Sounds good, happy to hear other opinions, so let this issue hang for a while.
I know it's used at Google for things where @testable
doesn't just work.
from promises.
@virl thank you for the discussion!
After discussing this further, we believe there are valid use cases where developers would want to use waitForPromises
for testing purposes. However, the preferred approach is still to leverage XCTest (i.e. XCTestExpectation, XCTNSPredicateExpectation, etc.) where possible.
Additionally, to make the intention of the waitForPromises
API in Swift more clear, we've limited the access level to internal
(i.e. @testable is required) and defined the API in Promise+Testing.swift
.
If you have any additional proposals to help make the API more clear, please feel free to submit a PR with the changes. Thank you!
from promises.
Related Issues (20)
- Sometimes Post Request Timeout HOT 6
- FBLPromises crashing HOT 2
- Question. Is it possible to change the delay time between retrys?
- Bug in iOS 16 DevBeta1 HOT 13
- Extremely slow compile time of Objective-C files
- Contextual type for closure argument list expects 1 argument, which cannot be implicitly ignored
- Objective-C Promises crashes in SwiftUI previews HOT 2
- ios 16 strings convertible to NSError HOT 1
- FBLPromises fails to load building macos flutter app HOT 2
- 'NSInvalidArgumentException', reason: '-[FBLPromise HTTPBody]: unrecognized selector sent to instance HOT 1
- Confused when retry a Promise variable
- type(of: value) is NSError.Type HOT 1
- Bitcode Issue on Xcode 13 when disable it. HOT 1
- Question regarding implementation HOT 3
- Ignore a certain Error and terminate Promise HOT 5
- Please release a new version of promises that includes latest changes for Firebase HOT 4
- Builds including PromisesSwift failing on CI
- Build fails for macOS with Xcode 14.3
- Create releases for new versions HOT 1
- The iOS deployment target 'IPHONEOS_DEPLOYMENT_TARGET' is set to 8.0, but the range of supported deployment target versions is 12.0 to 17.0.99.
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
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.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from promises.