Swizzle XCTestObservationCenter observer registration methods #278
Conversation
|
Could one of the brilliant @Quick/contributors kindly look this over? Quick's Xcode 7.3 support is waiting on this fix. |
| /// | ||
| /// @param originalSelector Original method to replace. | ||
| /// @param replacementSelector Replacement method. | ||
| + (void)swizzleSelector:(SEL)originalSelector withSelector:(SEL)replacementSelector { |
There was a problem hiding this comment.
Could you add the NMB_ prefix on here too? Or alternatively, turn it into a free function.
|
Hey @phatblat, thanks for moving forward with this! I'm left some comments on the implementation but I'm fine with the fundamental approach, as the least-evil solution we have available to us at this point in time. I'd love to hear input from @jeffh though, with his extensive experience monkey-patching ObjC. |
|
Looks good to merge! (modulo comments) |
|
Thanks for the feedback, @briancroom! I'll fix these up shortly. |
|
This is ready for re-review. |
|
Looks great 👍 Gonna leave it up to @briancroom to hit the 'merge' button. |
|
Cool, yeah this looks good. Sorry to nit, but would you mind squashing the commits @phatblat? Then go ahead and hit the button, I'd say. I was hoping I could do the squash myself but it looks like that isn't enabled for this repo. @modocache I think you or another admin would have to turn this on? |
Adding CurrentTestCaseTracker after XCTestLog has been registered.
|
Squashed. Just waiting for CI to finish and then I'll merge. |
|
@briancroom the squash option shows up after the first click on the merge button |
|
Aha, that explains it. Glad to see this fix go in. Thanks @phatblat! |
Updated installation docs
As discussed in #273, this approach uses swizzling of the private
_addLegacyTestObserver:method to registerCurrentTestCaseTracker, ensuring thatXCTestLoghas been registered first. The approach in #271 appeared to work for the typical use case, but Quick's test suite still had failing tests due to something not working right with these observers. This approach seems to work for all these cases.Addresses Quick/Quick#494. Redo of #271. May address #276. Hope to unblock Quick/Quick#507.