Skip to content

Modified meal #135

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

Open
wants to merge 1 commit into
base: 2022/python
Choose a base branch
from
Open

Conversation

kamilkrzyskow
Copy link

Fixed the no_output test to properly check if the output is empty.

Changed the input of no_output to 8:01 because the previous tests didn't check any value that went over a meal time with minutes only. Because of that, some students only used the str hours instead of following the specification to convert the str time to float.

Changed the order of execution for no_output and made other tests rely on the results of it, to prevent students from passing meal with a solution not per the specification.

@rongxin-liu rongxin-liu added the CS50P CS50 Python label Feb 6, 2024
@bxngyn bxngyn self-assigned this Jun 25, 2025
Copy link

@bxngyn bxngyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for submitting a PR!

We typically follow a standard template when writing checks, where we use check50.exists as the decorator for all the functions and check50.run....exit() to ensure valid execution. It seems you overrode all of the decorators with your new test_no_output function. The purpose of the function, which is to test for no output when given an invalid time, doesn't match the purpose of the other checks which is to test for valid output when given a valid time.

@bxngyn bxngyn requested a review from rongxin-liu June 25, 2025 15:20
@kamilkrzyskow
Copy link
Author

kamilkrzyskow commented Jun 25, 2025

It's been 3 years, so sorry if I come of as ignorant or forgetful.

@check50.check(previous) decorator blocks the execution of the decorated test if the previous test didn't pass. At the time of making PRs here my goal was to make the checker more "strict", so I moved the no_output test up the chain as it allowed to pass the assignment even without "perfect" code.

The idea was for the "strictness" of the assignment to lead the students to the proper "perfect" solution. I moved on from this ideology a wee bit over the years, as I now understand that there needs to be some leeway in the grading process to not destroy dreams 😸

So I guess this will be closed similarly to:

EDIT:
I see that there is a cycle to the PR pruning, which is close to ~summer time each year 😄

Thanks for having a look and taking the time to review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CS50P CS50 Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants