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

Pass args instead of null #538

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

Conversation

DavidCooperWCU
Copy link

This pass through method wasn’t passing necessary args for methods that take arguments. Now it does.

@bnmnetp
Copy link
Member

bnmnetp commented Sep 14, 2024

@DavidCooperWCU

What issue or problems does this PR address?

@bhoffman0 could your or Kate take a look at this and let me know what you think?

@bhoffman0
Copy link
Contributor

I'd like to get @kmcdonnell2 to look at it too, since it's her code. This is discussed more here https://discord.com/channels/1013815439161315348/1283143831067295766/1283874612572065927.

@DavidCooperWCU
Copy link
Author

@DavidCooperWCU

What issue or problems does this PR address?

When trying to get method output for instance methods that take arguments, without this fix, the codetesthelper will fail because the arguments are not sent to the method that requires arguments.

@bnmnetp
Copy link
Member

bnmnetp commented Sep 15, 2024

OK, this looks pretty innocuous, have you tested it ? Hopefully @kmcdonnell2 can give it a quick spin. Or maybe there is any easy way to test it on replit or something that is documented somewhere?

@DavidCooperWCU
Copy link
Author

This is a working test case. Tested on JGrasp, which is why the test class file is not RunestoneTest.java

WorkingExample.zip

@kmcdonnell2
Copy link

Yes, I believe this change works. I was able to do a quick verification on Repl.it.
https://replit.com/@kmcdonnell2/testCode

@bnmnetp
Copy link
Member

bnmnetp commented Sep 17, 2024

OK, I will merge, and this should be live with the next update to the site. Saturday at the latest.

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.

4 participants