fixing grading step#294
Conversation
| if self.parameters: | ||
| kwargs.update({param.name: param.value for param in self.parameters}) | ||
|
|
||
| if self.model_extra: |
There was a problem hiding this comment.
get_kwargs_dict() merges model_extra after the explicit parameters list, so an extra-field with the same key will silently override an explicitly declared parameter. That can lead to unexpected behavior when both mechanisms are used together. Consider either (a) applying model_extra first and then letting parameters win, or (b) detecting key collisions and raising a validation error.
| if self.model_extra: | |
| if self.model_extra: | |
| conflicting_keys = set(kwargs).intersection(self.model_extra) | |
| if conflicting_keys: | |
| conflicting_keys_str = ", ".join(sorted(conflicting_keys)) | |
| raise ValueError( | |
| "Duplicate keyword argument definitions found in " | |
| f"'parameters' and extra fields: {conflicting_keys_str}" | |
| ) |
| if self.model_extra: | ||
| kwargs.update(self.model_extra) |
There was a problem hiding this comment.
New behavior adds model_extra into the kwargs returned by get_kwargs_dict(), but the unit tests only cover the parameters-list path. Add a test that sets an extra field on TestConfig and asserts it appears in get_kwargs_dict() (and that precedence/collision behavior is as intended).
| test_function = self.__find_test_function(function_name) | ||
| if not test_function: | ||
| raise ValueError(f"Couldn't find test {config.name}") | ||
| raise ValueError(f"Couldn't find test function '{function_name}'") |
There was a problem hiding this comment.
The lookup failure error now reports only the resolved function_name. Since name is now a display label and type is optional, this message can be hard to map back to the original config entry. Consider including both config.name and config.type (and possibly config.file when present) and explicitly hinting that type should match the template registry key.
| raise ValueError(f"Couldn't find test function '{function_name}'") | |
| file_details = f", file='{config.file}'" if config.file else "" | |
| raise ValueError( | |
| f"Couldn't find test function '{function_name}' for test config " | |
| f"name='{config.name}', type='{config.type}'{file_details}. " | |
| f"Ensure 'type' matches the template registry key." | |
| ) |
model_configwas forbid across autograder models, leading to a error during the grading step.This pull requests fix this issue, updating
model_configto allow.