Add support for anonymous functions to assert_connected #793
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…string identifiers, allowing callables to be anonymous
bitwes
requested changes
Jan 5, 2026
Owner
bitwes
left a comment
bitwes
left a comment
There was a problem hiding this comment.
Thanks. These signal asserts are very unwieldy, and this is a good patch to get the new assert functionality.
Just a minor change to not replicate the ugliness of SignalAssertParameters.others. Changing _get_connection_info to return a dictionary might help refactoring that class later too (it needs it).
| return true | ||
| return false | ||
| push_error("Signal connection assertion called with bad signature. Read Docstring for correct signature.") | ||
| return [connected, signal_name, method_name, signal_object_name, method_object_name] |
Owner
There was a problem hiding this comment.
This should return a dictionary, an array is too fragile.
Comment on lines
+1449
to
+1454
| var conn_result = _get_connection_info(p1, p2, p3, p4) | ||
| var connected = conn_result[0] | ||
| var signal_name = conn_result[1] | ||
| var method_name = conn_result[2] | ||
| var signal_object_name = conn_result[3] | ||
| var method_object_name = conn_result[4] |
Owner
There was a problem hiding this comment.
Changing _get_connection_info to return a dictionary means we can use the dictionary values directly.
Comment on lines
+1474
to
+1479
| var conn_result = _get_connection_info(p1, p2, p3, p4) | ||
| var connected = conn_result[0] | ||
| var signal_name = conn_result[1] | ||
| var method_name = conn_result[2] | ||
| var signal_object_name = conn_result[3] | ||
| var method_object_name = conn_result[4] |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #786
Previously anonymous functions could not be tested for connection in assert_connected and assert_not_connected. The connection assertions relied on using the names of methods instead of references to them, which gets a little messy with anonymous function because they are all just named "<anonymous function>" (or something similar, I don't remember exactly what).
This PR implements a new system for testing signal/method connection that stores references to methods instead of just their names (and adds tests to make sure this behavior is well documented). The signature of the assertion methods was also a little bit complicated and untyped, and so I took the liberty of adding a push_error informing the user they have incorrectly called the assertion method in the case that they have. I can certainly remove this part if it is found unnecessary.