Skip to content

Updates Nimble submdule commit. #507

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

Merged
merged 1 commit into from
Apr 9, 2016
Merged

Conversation

ashfurrow
Copy link
Member

As discussed, we need to update this to get the latest fix for #494.

@modocache
Copy link
Member

Thanks!! Hopefully peeps aren't using the version of Nimble Quick's tests depend on as their Nimble dependency, but I have a feeling some do.

Feel free to merge once CI passes.

@ashfurrow
Copy link
Member Author

Hmm, both builds stalled out. Should I just try to restart?

@ashfurrow
Copy link
Member Author

Ha, looks like this happened the last time we upped the Nimble submodule commit: #505 I'll try restarting.

@ashfurrow
Copy link
Member Author

The same test is failing on both CI providers, probably not a coincidence... Since the Nimble change we're trying to bring in has to do with when test observation starts, I'm willing to bet it's a valid failure. I'll look into it and report back.

@ashfurrow
Copy link
Member Author

Tests didn't fail when running within Xcode. Invoking xcodebuild -workspace Quick.xcworkspace -scheme Quick-OSX clean test led to 🍏 tests. Looking into possible xctool issues.

@ashfurrow
Copy link
Member Author

Hmm, might be a trickier issue than expected. Kif ran into similar problems that Quick/Nimble#271 was supposed to fix, but decided not to use the solution we did dispatch_async. However, their solution appears to remove any call to addTestObserver:. I don't want to ask for clarification there without a better understanding of what XCTestObservationCenter is and why we need it.

Suggestions on how to proceed would be awesome 🙇 I feel a little out of my depth.

@phatblat
Copy link
Member

Looks like KIF's solution was to move their test observation functionality into the KIFTestCase.setUp method to start before each test. Nimble doesn't have the an XCTestCase subclass to move its logic to.

The Nimble test observation logic appears to only be used when _runtime(_ObjC) so that it can testCase.recordFailureWithDescription instead of XCTFail on the swift side.

@ashfurrow
Copy link
Member Author

Hmm. You reckon its worth opening an issue on Nimble to discuss options? Maybe there's another hook we could use, and dispatch_once to add the observer.

@phatblat
Copy link
Member

Yes, let's open a Nimble issue to discuss. I've been doing some debugging and have a theory.

@phatblat
Copy link
Member

phatblat commented Apr 7, 2016

@ashfurrow can you update your branch to Quick/Nimble@1a9e53e?

@ashfurrow
Copy link
Member Author

Done 👍 Let's see how CI goes 😄

@ashfurrow
Copy link
Member Author

@phatblat
Copy link
Member

phatblat commented Apr 9, 2016

Aw yeah! 🚀

@phatblat phatblat merged commit 6e34d53 into Quick:master Apr 9, 2016
@briancroom
Copy link
Member

Awesome! Thanks guys 🙌

@phatblat
Copy link
Member

phatblat commented Apr 9, 2016

Time for a release?

@modocache
Copy link
Member

Sure! @phatblat, I've added you as a pod trunk owner, so you should be able to tag a release. Use script/release to do so 👍

@briancroom
Copy link
Member

There's also the nice section in the Contributing document about making releases: https://github.com/Quick/Quick/blob/master/CONTRIBUTING.md#creating-a-release

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 this pull request may close these issues.

4 participants