Skip to content

pangram: Add case-insensitive test#439

Merged
Insti merged 1 commit intoexercism:masterfrom
behrtam:add-test-pangram
Nov 27, 2016
Merged

pangram: Add case-insensitive test#439
Insti merged 1 commit intoexercism:masterfrom
behrtam:add-test-pangram

Conversation

@behrtam
Copy link
Copy Markdown
Contributor

@behrtam behrtam commented Nov 6, 2016

see #266

Comment thread exercises/pangram/canonical-data.json Outdated
"expected": false
},
{
"description": "test that count is case-insensitive",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This needs a better description of what we're testing for.
case insensitivity is already covered by: "Five quacking Zephyrs jolt my wax bed."

Copy link
Copy Markdown
Contributor

@Insti Insti Nov 6, 2016

Choose a reason for hiding this comment

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

How about: "upper and lower case versions of the same character should not be counted separately"

Edit: I think there's a better description than that, I just don't know what it is right now.

@kytrinyx
Copy link
Copy Markdown
Member

kytrinyx commented Nov 8, 2016

@behrtam Here also a more descriptive commit message would be really helpful.

@behrtam behrtam force-pushed the add-test-pangram branch 3 times, most recently from a421ffe to 7e21769 Compare November 23, 2016 10:39
Copy link
Copy Markdown
Contributor

@Insti Insti left a comment

Choose a reason for hiding this comment

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

The test description comments from my previous review have not been addressed.

@behrtam
Copy link
Copy Markdown
Contributor Author

behrtam commented Nov 27, 2016

I think there's a better description than that, I just don't know what it is right now.

Any new ideas or someone else?

@Insti
Copy link
Copy Markdown
Contributor

Insti commented Nov 27, 2016

No new ideas, "upper and lower case versions of the same character should not be counted separately" is the best I have for now.

@stkent
Copy link
Copy Markdown
Contributor

stkent commented Nov 27, 2016

+1 to "upper and lower case versions of the same character should not be counted separately"; it's a little verbose, but perfectly clear.

The new test cases verifiys that the characters are
counted case-insensitive.

Resolves: exercism#266
@behrtam
Copy link
Copy Markdown
Contributor Author

behrtam commented Nov 27, 2016

Perfect, thanks both of you. Than I guess we are good to go.

@Insti Insti merged commit f04fb3d into exercism:master Nov 27, 2016
@Insti
Copy link
Copy Markdown
Contributor

Insti commented Nov 27, 2016

Thanks @behrtam ❤️

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.

4 participants