google / promises

Promises is a modern framework that provides a synchronization construct for Swift and Objective-C.
Apache License 2.0
3.81k stars 295 forks source link

Increase waitForPromise timeout in Testing section #54

Closed SRozhina closed 6 years ago

SRozhina commented 6 years ago

Hi, I've just used Promises in unit tests with Xcode 9.4.1 (MacBook Air 2016). I tried to use your example and it didn't work for me:

@testable import Promises

// ...
func testExample() {
  // Arrange & Act.
  let promise = Promise<Int> { 42 }

  // Assert.
  XCTAssert(waitForPromises(timeout: 1))
  XCTAssertEqual(promise.value, 42)
  XCTAssertNil(promise.error)
}
// ...

I got an assertion error on the waitForPromises line. It is strange because there is no operations that could take up to 1 second. Anyway when I tried to increase the timeout to waitForPromises(timeout: 2) the test was passed.

Am I missing something here? If this is expected behaviour, here is PR that updates the timeout in docs

googlebot commented 6 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers
shoumikhin commented 6 years ago

Hi Sophia, thank you for the heads up!

Does that happen when you run that test exclusively? Does it happen consistently with 1 second timeout, but when you increase it everything passes?

SRozhina commented 6 years ago

Thank you for so quick response, Yes, it happens when I run the test exclusively. And the test passes when I increase the timeout.

What is more interesting is when I run the following test suite (with two similar tests) first test gets "red" and the second gets "green":

class TestSuite: XCTestCase {
    func testExample() {
        // Arrange & Act.
        let promise = Promise<Int> { 42 }

        // Assert.
        XCTAssert(waitForPromises(timeout: 1))
        XCTAssertEqual(promise.value, 42)
        XCTAssertNil(promise.error)
    }

    func testExample2() {
        // Arrange & Act.
        let promise = Promise<Int> { 42 }

        // Assert.
        XCTAssert(waitForPromises(timeout: 1))
        XCTAssertEqual(promise.value, 42)
        XCTAssertNil(promise.error)
    }
}

jul-08-2018 23-44-29

shoumikhin commented 6 years ago

Hi Sophia, I'm still unable to reproduce your issue. May you please zip an Xcode project with all sources where the tests fail for you and attach here? Thanks!

SRozhina commented 6 years ago

I've tried to create new test project and run tests from previous comment. And they were passed. So I think there is conditions for tests' failure in my project. You could try to reproduce. Try to run one of the next tests from FullEventPresenterTests -> testPresenterTryToGetSimilarEventWhenTheyAreAlreadyFetched or testPresenterTryToFetchEventsWhereThereIsNoSimilar

shoumikhin commented 6 years ago

Hi Sophia,

All tests in your project passed for me with 1 sec timeout on Xcode 9.4.0, except testPresenterAllEventsLoaded, which requires at least 2 secs to wait for promises created in presenter.setup() since you explicitly asyncAfter 2 secs in searchEvents.

By the way, instead of that Cancelable wrapper you may consider extending a Promise class itself, like so:

enum CancellableErrors: Error {
  case cancelled
}

extension Promise {
  public func cancel() {
    reject(CancellableErrors.cancelled)
  }
}

Just so you can then use the promise directly anytime.

Let us know if you still have any issues with waitForPromises(). Thanks!

ghost commented 6 years ago

Closing for now. Please reopen if this is still an issue. Thank you!