Remove performSelector spy

[ggilles]

This is the result of trying to fix https://github.com/wix/Detox/issues/4194

I tracked down the issue to the fact that DetoxSync swizzles both NSObject:performSelector:withObject:afterDelay:inModes: and NSObject:performSelector:onThread:withObject:waitUntilDone:modes:, but it does not swizzle the associated cancel methods NSObject:cancelPreviousPerformRequestsWithTarget.

This means that any app/library using cancelPreviousPerformRequestsWithTarget will perform differently when run using detox. This is the case for the single-tap handling of react-native-gesture-handler and the result is that only the first single tap is correctly detected when run using detox.

My proposal here is to simply remove the performSelector spy. It doesn't seem to affect the results of the Detox e2e test-suite.

[noomorph]

Hmm, this is truly interesting, but I have concerns.

Do you know what idling resources are and what they're for?

I mean, the fact that this removal does not break the Detox E2E suite:

1) doesn't guarantee that flakiness has not increased — that can be measured only through hundreds of test runs 2) doesn't guarantee that other projects using Detox won't get less reliable.

Can we look in the opposite direction, and swizzle cancelPreviousPerformRequestsWithTarget instead?

[ggilles]

@noomorph I think I understand why those resources were tracked initially, yes: The role of DetoxSync is to be able to tell when the device is busy or not, so that Detox can wait for the device to be idle before dispatching the next expectation/action.

And it's true that this is quite a bold move. I suggested that change because effectively Native timers are already no longer properly tracked for iOS16+. This suggests that tracking those resources might not be so important in practice.

However, I just realized my runs of the Detox E2E suites were wrong: the DetoxSync code is not compiled at all when doing npm build:ios from the detox/test folder, and lerna bootstrap doesn't compile DetoxSync either: I checked this by introducing an obvious compile error in DetoxSync and couldn't make compilation fail from the Detox folder... 🤦

So, it seems I should go back to testing this proper.

Can you tell how to make Detox use a version of DetoxSync built from my branch ?

[noomorph]

Well, DetoxSync is a git submodule inside Detox, so you'd just have to set it to a correct revision, AFAIU.

@asafkorem could you assess the situation around this PR, please, and recommend something to @ggilles. I clearly see that his observation is trustworthy, and some action about swizzling is indeed required here.

[asafkorem]

@noomorph I think I understand why those resources were tracked initially, yes: The role of DetoxSync is to be able to tell when the device is busy or not, so that Detox can wait for the device to be idle before dispatching the next expectation/action.

And it's true that this is quite a bold move. I suggested that change because effectively Native timers are already no longer properly tracked for iOS16+. This suggests that tracking those resources might not be so important in practice.

However, I just realized my runs of the Detox E2E suites were wrong: the DetoxSync code is not compiled at all when doing npm build:ios from the detox/test folder, and lerna bootstrap doesn't compile DetoxSync either: I checked this by introducing an obvious compile error in DetoxSync and couldn't make compilation fail from the Detox folder... 🤦

So, it seems I should go back to testing this proper.

Can you tell how to make Detox use a version of DetoxSync built from my branch ?

Hey @ggilles, if you're testing Detox with our test app, you can run detox rebuild-framework-cache. This will rebuild the framework (including DetoxSync - of course, you need to make sure the git submodule is pointing on the relevant DetoxSync commit).

[asafkorem]

Hey @ggilles, I'll begin looking into this issue today. I'm uncertain whether I'll adopt a similar method (removing part of our synchronization mechanism), but I'll likely attempt to sync with the calls we've missed

It would be greatly beneficial if you could replicate this issue within a specific branch of our test-app. Including a failing test due to this issue will make it much easier. I'm curious whether it’s possible to reproduce this without integrating the gesture-handler library—could it be straightforward enough?

[ggilles]

@asafkorem I think it should be pretty straightforward to create a test for this, I am just short on time right now.

It should be something along the lines of:

- (void)triggerError
{
    [self makeErrorVisible];
}

// This would be wired to a UI element so that Detox can fire it
- (void) triggerErrorThenCancel
{
    // ask to trigger the error UI
    [self performSelector:@selector(triggerError) withObject:nil afterDelay:1];
    // and cancel that immediately
    [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(triggerError) object:nil];
}

And then on the Detox side you would trigger triggerErrorThenCancel and then expect that the error isn't visible.

[asafkorem]

@ggilles, I was able to reproduce that. I will push a workaround specifically made for the RNGH case until we can address this with a more generic solution. Unfortunately, the future fix is not trivial.

[asafkorem]

Here's the workaround: https://github.com/wix-incubator/DetoxSync/pull/69. I will release a Detox version with this patch in the next couple of hours.