Skip to content
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

Add missing tests #4

Merged
merged 2 commits into from
Jul 26, 2023
Merged

Conversation

Diogo-Rossi
Copy link
Contributor

Add tests required in comment originally posted by @bluebird75 in #3 (comment)

@Diogo-Rossi Diogo-Rossi force-pushed the add-missing-tests branch 2 times, most recently from bdfa7bb to 898114d Compare April 23, 2023 14:26
@bluebird75
Copy link
Collaborator

Thanks for the commit. Can you also make the CI green with your additions ? That's the point of the tests ... they should succeed

@bluebird75
Copy link
Collaborator

Can you rebase on the latest main ? That should make your tests successful

@Diogo-Rossi
Copy link
Contributor Author

Can you rebase on the latest main ? That should make your tests successful

Done.

Sorry about not investigating the tests. I've been out of time lately.

@bluebird75
Copy link
Collaborator

I'll merge as it is and fix it afterwards

@bluebird75 bluebird75 merged commit 07b4311 into python-qt-tools:main Jul 26, 2023
0 of 2 checks passed
@bluebird75
Copy link
Collaborator

Ok, you highlighted a mistake of mine. It took me several days to get the signature of Slot() right so that it typechecks correctly the method arguments and return value. But ... I did my tests on function slots instead of class methods. And it makes a big difference, thanks for highlighting it.

It is now fixed.

@Diogo-Rossi
Copy link
Contributor Author

Ok, you highlighted a mistake of mine. It took me several days to get the signature of Slot() right so that it typechecks correctly the method arguments and return value. But ... I did my tests on function slots instead of class methods. And it makes a big difference, thanks for highlighting it.

It is now fixed.

Thank you for the observation.

I also had a look in the error messages and saw something that may be important.

The first error tells that there is no overload that matches the types, but it seems not catching the correct signatures of "static methods" (lines [114], [118], [119])

[113]tests\signal_slot.py:13: note: Possible overload variants:
[114]tests\signal_slot.py:13: note: def connect(arg__2: str, arg__3: Callable[..., Any], type: ConnectionType = ...) -> bool
[115]tests\signal_slot.py:13: note: def connect(self, arg__1: str, arg__2: Callable[..., Any], type: ConnectionType = ...) -> bool
[116]tests\signal_slot.py:13: note: def connect(self, arg__1: str, arg__2: QObject, arg__3: str, type: ConnectionType = ...) -> bool
[117]tests\signal_slot.py:13: note: def connect(self, sender: QObject, signal: str, member: str, type: ConnectionType = ...) -> Connection
[118]tests\signal_slot.py:13: note: def connect(signal: QMetaMethod, receiver: QObject, method: QMetaMethod, type: ConnectionType = ...) -> Connection
[119]tests\signal_slot.py:13: note: def connect(signal: str, receiver: QObject, member: str, type: ConnectionType = ...) -> Connection
[120]Found 1 error in 1 file (checked 1 source file)

By changing the line (static method):

self.connect(self,SIGNAL('clicked()'),self.my_slot_no_arg)

to

self.connect(SIGNAL('clicked()'),self.my_slot_no_arg)

I got the test passing in local environment

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.

2 participants