-
-
Notifications
You must be signed in to change notification settings - Fork 9
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
Representer Updates and Golden Tests #45
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
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.txt
for each test, as well as the creation of a code file and a
config.json
for each test. which adds up to ~1, 002 files that only need "spot checking", if any checking at all.There are 10
example
tests, 22concept exercise
tests, and 135practice exercise
tests. Theexample
tests 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
,concept
andpractice
exercise groups.config.json
files for each test.mapping.json
,representatin.json
,representation.out
, andrepresentation.txt
test/test_representer.py
)run-tests-in-docker.sh
scriptrepresentation.json
metadata output file (editedrepresenter/__init__.py
)astor
library dependency and converted to using Python AST module (editedtest/utils.py
).dockerignore
python:3.11.5-alpine3.18
run.sh
andrun-in-docker.sh
scriptsrepresentation.txt
to be an AST stringnormalizer.py
to remove__main__
blocks andprint()
statements.normalizer.py
to handle generators.__init__.py
to 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.txt
to the Python content repo.