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

Added "Shinychu" card for testing wrong shiny evaluation #504

Merged
merged 2 commits into from
Jul 5, 2024

Conversation

Rocche
Copy link
Contributor

@Rocche Rocche commented Jul 4, 2024

[no important files changed]

Hey!

This pull request adds a (useless) not covered case in Gotta Snatch 'Em All` exercise, which is the "fake shiny" card (feel free to name the case the way you like).

Basically the instructions of the last part of the exercise states:

Implement shiny_cards, which takes a collection and returns a set containing all the cards that start with "Shiny ".

That means that cards starting with "Shiny" without a space should not be considered shiny cards. For example, the totally made up card Shinychu should not be considered as shiny. However in the current state of the exercise I could write a function that filters for strings starting with "Shiny" and the tests pass.

To correct this behavior, I added the Shinychu card to all the tests regarding shiny cards. For example:

pub fn shiny_cards_with_none_test() {
  gotta_snatch_em_all.shiny_cards(
    set.from_list(["Blasturtle", "Zumbat", "Hitmonchuck", "Shinychu"]),  // <-- here the added fake shiny card
  )
  |> should.equal(set.new())
}

This is probably a useless correction, but I think it better completes the exercise.

PS: this is a retake of a previous pull request, but with the [no important files changed] string added to the commit body since there is no need at all to re-test all students' submissions.

Copy link

github-actions bot commented Jul 4, 2024

This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested.

If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos.

[no important files changed]

For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping @exercism/maintainers-admin in a comment. Thank you!

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Could you add a new test rather than modifying the existing ones please

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

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

Thank you!

@lpil lpil merged commit 67032a4 into exercism:main Jul 5, 2024
2 checks passed
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 this pull request may close these issues.

2 participants