update `game` valid level check and monkey-patch

[kamilkrzyskow]

Updated the level check to check for the lowest valid value. A student used random.randrange(level) which starts from 0. Later in the

def test_integer_guess():
    """game.py rejects out-of-range guess"""

It tired to get a re-prompt after setting the level of 1 and guessing 0. But with random.randrange(level) a level of 1 means that it generated 0 as the number to guess, so the guess was correct.

EDIT: After writing out the explanation above, I've noticed that the tests didn't properly test upper bound level ranges too. I've copied the random.seed(0) testing approach from another problem set and adjusted the tests accordingly.

[kamilkrzyskow]

Sorry for the typos, ~~if you still accept this PR then please squash all of those commits.~~ EDIT: So I've mistook this conversation feed for the commit feed, I'm OK with 2 commits, I thought that the force-pushed commit duplicated the previous one. Sorry for the confusion.

[kamilkrzyskow]

There is also the issue with:

import game
try:
   game.main()
except AttributeError:
   pass

that I've addressed in https://github.com/cs50/problems/pull/123, but I still wait on feedback on this matter.

[CarterZenke]

Thanks, @kamilkrzyskow, we'll be able to review by early next week.

[kamilkrzyskow]

I understand that #123 might change the testing too much, but this game update still seems reasonable🤔

[CarterZenke]

I appreciate the thoughtfulness (and patience) @kamilkrzyskow! @rongxin-liu, I'm going to opt to close without merging for now: for this scenario, I think it's better to explicitly which random number will be chosen (4), rather than relying on random.seed, which doesn't make it immediately clear which number is the correct choice. And I could be misinterpreting, but I'm not sure that this update:

@check50.check(exists)
def test_valid_level():
    """game.py accepts valid level"""
    check50.run("python3 game.py").stdin("1", prompt=True).stdout(regex("Guess"), "Guess:", regex=True).kill()

substantially improves the current:

@check50.check(exists)
def test_valid_level():
    """game.py accepts valid level"""
    check50.run("python3 game.py").stdin("10", prompt=True).stdout(regex("Guess"), "Guess:", regex=True).kill()

since we're just checking to see if the game accepts any number >= 1.

[kamilkrzyskow]

Thanks again for the review @CarterZenke Just like with https://github.com/cs50/problems/pull/123 the purpose of this was to increase the strictness.

The student could use an input > 1 condition instead of input >= 1, so they would get a pass on test_valid_level that only tests with 10 instead of the lowest possible value 1, but would not get a pass on another that inputs 1 as the level, so this would lead to confusion (and perhaps it did lead to confusion as per the chats on Discord, can't really remember after 2 years).

I see this task got some more updates over the last few months, like the non-positive rejection 👍