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

Replace AsyncTestHelper with XCTestExpectation #2200

Merged
merged 6 commits into from Aug 7, 2014

Conversation

Projects
None yet
5 participants
@astralbodies
Member

astralbodies commented Aug 6, 2014

Closes #1613

Brings all unit tests green and removes the former AsyncTestHelper class which was fundamentally flawed since it was using a shared global variable to store the semaphore. Now every asynchronous test uses an XCTestExpectation instance and if the expectation needs to be fulfilled by the main context saving, it is set on CoreDataTestHelper.testExpectation.

This requires Xcode 6 beta 5 or greater to execute.

2014-08-06_10-07-19

@astralbodies astralbodies added the testing label Aug 6, 2014

@astralbodies astralbodies added this to the 4.4 milestone Aug 6, 2014

@astralbodies

This comment has been minimized.

Show comment
Hide comment
@astralbodies

astralbodies Aug 6, 2014

Member

cc: @sendhil since you wanted to get this project building in Travis CI

Member

astralbodies commented Aug 6, 2014

cc: @sendhil since you wanted to get this project building in Travis CI

@michaelbeil

This comment has been minimized.

Show comment
Hide comment
@michaelbeil

michaelbeil Aug 7, 2014

Looking great.

michaelbeil commented Aug 7, 2014

Looking great.

@astralbodies

This comment has been minimized.

Show comment
Hide comment
@astralbodies

astralbodies Aug 7, 2014

Member

@koke @jleandroperez how does this look to you guys?
@h4xnoodle Looping you in since you helped create the first semaphore-based solution and thought you'd like to see how we've iterated!

Member

astralbodies commented Aug 7, 2014

@koke @jleandroperez how does this look to you guys?
@h4xnoodle Looping you in since you helped create the first semaphore-based solution and thought you'd like to see how we've iterated!

@koke

This comment has been minimized.

Show comment
Hide comment
@koke

koke Aug 7, 2014

Member

This looks much better 👏 👏
Using semaphores seemed like a good idea at the time, but we outgrew it quite fast

Member

koke commented Aug 7, 2014

This looks much better 👏 👏
Using semaphores seemed like a good idea at the time, but we outgrew it quite fast

@astralbodies

This comment has been minimized.

Show comment
Hide comment
@astralbodies

astralbodies Aug 7, 2014

Member

I agree @koke - I think it was fine for Core Data alone but then using the same mechanism in other async tests starting showing the limitations.

Member

astralbodies commented Aug 7, 2014

I agree @koke - I think it was fine for Core Data alone but then using the same mechanism in other async tests starting showing the limitations.

@jleandroperez

This comment has been minimized.

Show comment
Hide comment
@jleandroperez

jleandroperez Aug 7, 2014

Contributor

@astralbodies looking just great!!

:shipit:

Contributor

jleandroperez commented Aug 7, 2014

@astralbodies looking just great!!

:shipit:

astralbodies added a commit that referenced this pull request Aug 7, 2014

Merge pull request #2200 from wordpress-mobile/issue/1613-test-expect…
…ations

Replace AsyncTestHelper with XCTestExpectation

@astralbodies astralbodies merged commit 1a64423 into develop Aug 7, 2014

@astralbodies astralbodies deleted the issue/1613-test-expectations branch Aug 7, 2014

@sendhil

This comment has been minimized.

Show comment
Hide comment
@sendhil

sendhil Aug 7, 2014

Contributor

loki yeahhhh

Contributor

sendhil commented Aug 7, 2014

loki yeahhhh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment