-
-
Notifications
You must be signed in to change notification settings - Fork 11
Representer Updates and Golden Tests #45
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
Conversation
Changed representation.txt to be a non-indented AST string Changed representation.out to have AST converted to code Added Normalizers for print removal, __main__ removal, and generator support
ErikSchierboom
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! One small question: does the test runner handle multiple submitted files? As in: if a student submits both leap.py and leap_helper.py, will both files be used in the representation?
Good Question. Short answer: no on the representation, Except for 'Cater Waiter', where we provide a helper file that is also present in the editor,
So I thought in the past that the whole solution folder was copied, but no longer. So the import scenario from above no longer works -- the runner does not process multiple files, and neither does the rest of the toolchain. The one exception is the (exercisim-provided) 'Caiter Waiter' data file. But its not considered "part" of the student solution - only required for it (so already in the Docker container).
|
That might be a good idea :) It's not high priority or anything, as the vast majority of solutions will not add new files, but in theory it is possible. I expect it to be more likely for more complex exercises, where extracting code to different files can help structure things a bit nicer.
Yeah, so the logic to use probably looks something like this:
Do you want to have a go at adding this before we merge this? |
|
Sure - I can give it a go! 😄 I'll need to do the runner and analyzer as well. But I think I will save the Analyzer for later (I have a bunch of stuff to do for it -including using Alpine and making golden tests- so its going to be a while). |
|
Alrighty! I think were good to go...but maybe I will have you merge this, so that I don't automatically trigger something catastrophic? I renamed the job to |
|
Just to be clear: this will change the representation output and thus require the existing representations to be re-run? |
Yup. It will. 😢 Specifically:
Edited to add: Since we've replaced |
|
We'll re-run the representations shortly (it'll take a while). |
This appears at first glance to be a horrifying amount of files changed. Don't be alarmed. 😉
The bulk of the "changes" are for the 167 new Golden tests that required "recording" :
mapping.json,representation.json,representation.out, andrepresentation.txtfor each test, as well as the creation of a code file and a
config.jsonfor each test. which adds up to ~1, 002 files that only need "spot checking", if any checking at all.There are 10
exampletests, 22concept exercisetests, and 135practice exercisetests. Theexampletests try to get directly at normalization "issues", whereas the rest of the tests are using exemplar/example code copied from the content repo.The remaining files are the other non-test case changes listed below. I am least confident in the CI/workflow changes, and could use a going-over on the shell scripts as well. 😄
Due to the changes in normalization and
representation.txt, all representations for Python will need to be re-run.example,conceptandpracticeexercise groups.config.jsonfiles for each test.mapping.json,representatin.json,representation.out, andrepresentation.txttest/test_representer.py)run-tests-in-docker.shscriptrepresentation.jsonmetadata output file (editedrepresenter/__init__.py)astorlibrary dependency and converted to using Python AST module (editedtest/utils.py).dockerignorepython:3.11.5-alpine3.18run.shandrun-in-docker.shscriptsrepresentation.txtto be an AST stringnormalizer.pyto remove__main__blocks andprint()statements.normalizer.pyto handle generators.__init__.pyto create AST without typehints and without indentation.There are probably more small changes I've missed due to the fact that this took a really long time to complete. Please let me know if you have any questions or issues.
I will follow up with a small PR to change the README after merge, as well as to add
representations.txtto the Python content repo.