Skip to content
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

[Bowling] Exception-expecting tests pass themselves by accident #2839

Closed
rneilsen opened this issue Jan 4, 2022 · 1 comment · Fixed by #2840
Closed

[Bowling] Exception-expecting tests pass themselves by accident #2839

rneilsen opened this issue Jan 4, 2022 · 1 comment · Fixed by #2840

Comments

@rneilsen
Copy link
Contributor

rneilsen commented Jan 4, 2022

(status: I am currently working on learning how to fix, and then fixing, this bug :) Pull request submitted!)

The following tests are supposed to be checking that they get an exception when they ask for the score when a game is incomplete for various reasons:

  • test_an_unstarted_game_cannot_be_scored
  • test_an_incomplete_game_cannot_be_scored
  • test_bonus_rolls_for_a_strike_in_the_last_frame_must_be_rolled_before_score_can_be_calculated
  • test_both_bonus_rolls_for_a_strike_in_the_last_frame_must_be_rolled_before_score_can_be_calculated
  • test_bonus_roll_for_a_spare_in_the_last_frame_must_be_rolled_before_score_can_be_calculated

However, all five of these tests make a call to game.roll() without passing any arguments. game.roll() is supposed to be passed a number of pins on a roll; when that argument is missing, it raises a TypeError. Those five tests, which were expecting an exception to be raised, therefore pass - but the exception that was raised was nothing to do with the game being complete or incomplete (which the tests are supposed to be checking for); the exception was actually just a consequence of the malformed call to the roll method.

Comparing the Python test file with the Ruby test file for the same exercise shows that the Ruby ones call the game.score method, not the game.roll method, for those five tests, which indicates that the problem is actually in the Python test file generator (JinJa2 template), specifically this block of code (lines 7-10:

        with self.assertRaisesWithMessage(Exception):
            game.roll({{ input["roll"] }})
        {% else -%}

In error_case type tests, there is no consideration of the property field from the canonical-data.json file; so the tests that expect an exception to be raised are always generated with game.roll() with no arguments, and thus they always do generate exceptions, and thus they always pass, regardless of what the user has coded.

Contrast with the corresponding code in the Ruby test file generator, lines 31-35, which correctly use the property field to decide which function call should be used for these tests.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 4, 2022

🤖   🤖

Hi! 👋🏽 👋 Welcome to the Exercism Python Repo!

Thank you for opening an issue! 🐍  🌈 ✨


  •   If you are requesting support, we will be along shortly to help. (generally within 72 hours, often more quickly).
  •   Found a problem with tests, exercises or something else??  🎉
      ◦ We'll take a look as soon as we can & identify what work is needed to fix it. (generally within 72 hours).

​          ◦ If you'd also like to make a PR to fix the issue, please have a quick look at the Pull Requests doc.
             We  💙  PRs that follow our Exercism & Track contributing guidelines!

  •   Here because of an obvious (and small set of) spelling, grammar, or punctuation issues with one exercise,
      concept, or Python document?? 🌟 Please feel free to submit a PR, linking to this issue. 🎉
    ‼️  Please Do Not ‼️

    ​        ❗ Run checks on the whole repo & submit a bunch of PRs.
                  This creates longer review cycles & exhausts reviewers energy & time.
                  It may also conflict with ongoing changes from other contributors.
    ​        ❗ Insert only blank lines, make a closing bracket drop to the next line, change a word
                  to a synonym without obvious reason, or add trailing space that's not an EOL for the very end of text files.
            ❗ Introduce arbitrary changes "just to change things" .

            ...These sorts of things are not considered helpful, and will likely be closed by reviewers.

  • For anything complicated or ambiguous, let's discuss things -- we will likely welcome a PR from you.
  • Here to suggest a feature or new exercise?? Hooray! Please keep in mind Chesterton's Fence.
    Thoughtful suggestions will likely result faster & more enthusiastic responses from maintainers.

💛  💙  While you are here... If you decide to help out with other open issues, you have our gratitude 🙌 🙌🏽.
Anything tagged with [help wanted] and without [Claimed] is up for grabs.
Comment on the issue and we will reserve it for you. 🌈 ✨

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

Successfully merging a pull request may close this issue.

1 participant