Add unit tests for GSFFIInvocation

[qmfrederik]

Selector forwarding from one object to another seems to be broken when using libobjc2 on Windows (see also: gnustep/plugins-themes-WinUXTheme#7 (comment)).

This PR contains two tests which both pass on Linux, pass on Windows when using the GCC runtime, and fail on Windows when using libobjc2 to demonstrate the problem (and verify a fix).

[fredkiefer]

You should move this line before the test line, otherwise you never see the output for test failures and only there it is relevant.

[fredkiefer]

Thank you for this test case, this seems very interesting. What I don't understand is, why only one of the tests seems to fail. Shouldn't both behave the same? Sorry, my bad, they both fail, so having just one test case would be enough.

[fredkiefer]

In the else case you should call the super implementation, to make sure we end up in -doesNotRecognizeSelector:. This won't change the problem, just document the correct way to implement this method.

[fredkiefer]

@davidchisnall could you please have a look at this issue? The most likely cause seems to be a difference in the bevahiour of libobjc2 on Windows.

[qmfrederik]

Thanks @fredkiefer , I made the changes you requested.

You are correct: both tests fail. We can probably keep the GSFakeNSString test and do away with the GSFakeNSMenuItem test. There's a subtle difference in both tests -- in the GSFakeNSString case, selectors are forwarded to a type which is defined in another module; in the GSFakeNSMenuItem both types are defined in the same module. I don't think it matters, though, so one test is probably enough.

[qmfrederik]

For completeness: If, in GSFFInvocation.m, in the load method, I comment out the objc_proxy_lookup = gs_objc_proxy_lookup; line, everything works as expected.

[fredkiefer]

You most likely also get a working result if you don't implement the method -forwardingTargetForSelector:, which is a speed up for libobjc2.

[qmfrederik]

You most likely also get a working result if you don't implement the method -forwardingTargetForSelector:, which is a speed up for libobjc2.

That's correct; I split the test in two -- one which implements forwardingTargetForSelector and another which implements forwardInvocation and methodSignatureForSelector.

[rfm]

These look good to me.

[qmfrederik]

This has been fixed in libobjc2 (gnustep/libobjc2#313) and the fix is being backported to MSYS (msys2/MINGW-packages#22538). I suggest we wait for MSYS to get the libobjc2 fix, after which CI should be green(er) again.

[triplef]

Thanks for adding these tests @qmfrederik. Did MSYS already get the libobjc2 fix so this could be merged?

[qmfrederik]

@triplef I don't think these fixes made it to MSYS yet. We could backport this fix to MSYS. The last libobjc2 release was in March, and there have been a bunch of fixes since, so perhaps it's time to cut a new libobjc2 release instead?

[triplef]

Thanks for the update. Either way, I think it’s fine to wait too or alternatively we could mark the test as hopeful on MSYS to unblock merging this.

[qmfrederik]

@triplef The MSYS2 PR has been merged (msys2/MINGW-packages#22538), the package is now sitting in the queue: https://packages.msys2.org/queue

[triplef]

It looks like the libobjc2 MinGW package was updated to libobjc2-2.2.1-2 (from libobjc2-2.2.1-1), and now this test is failing in CI – any idea?

base/Functions/runtime.m:
runtime.m:131 ... class_respondsToSelector() works for class method
runtime.m:133 ... class_respondsToSelector() works for superclass method

[triplef]

FYI @gcasa marked these runtime tests as hopeful on Windows in e8181d3. It’s passing on MSVC though.

[gcasa]

FYI @gcasa marked these runtime tests as hopeful on Windows in e8181d3. It’s passing on MSVC though.

These tests NEED to pass. I marked them as hopeful so that CI would be useful for detecting other issues. There is no reason why it should always be RED. Can we please either get it to the point where these pass?