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

test: add unit test for binding arguments to static-method callables #1029

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arocull
Copy link

@arocull arocull commented Jan 31, 2025

I was having some issues with binding arguments to static method Callables. @Bromeon suggested I make a PR to add some unit tests.

I'm new to contributing and I don't fully understand the Rust toolchain, so looking for feedback.

  • Should I wrap this test in the cfg!(since_api="4.4") seeing as the callable API has changed a bit in that Godot version? How can I run this test with that API if so?
  • Test fails, indicating a bug. Should I just, comment it out to prevent interrupting workflows? Commented it out for now

@arocull arocull force-pushed the static-callable-test branch 2 times, most recently from 6384178 to 7e11a42 Compare February 1, 2025 18:07
@arocull arocull changed the title WIP: test: add unit test for binding arguments to static-method callables test: add unit test for binding arguments to static-method callables Feb 1, 2025
@arocull arocull marked this pull request as ready for review February 1, 2025 18:08
@Bromeon Bromeon added bug c: core Core components labels Feb 1, 2025
@Bromeon Bromeon force-pushed the static-callable-test branch from 7e11a42 to abe5667 Compare February 2, 2025 19:32
@arocull arocull force-pushed the static-callable-test branch 5 times, most recently from 309976d to bae2387 Compare February 6, 2025 21:51
@arocull
Copy link
Author

arocull commented Feb 12, 2025

This is complete I think, unless there is anything I should change or if I need to wait on anything.
Rebasing.

@arocull arocull force-pushed the static-callable-test branch from bae2387 to 25d3c39 Compare February 12, 2025 00:12
@Bromeon
Copy link
Member

Bromeon commented Feb 12, 2025

Thanks! Will try to reserve some time in the next couple of days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: core Core components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants