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

add pangram tests #228

Merged
merged 1 commit into from
Apr 23, 2016
Merged

add pangram tests #228

merged 1 commit into from
Apr 23, 2016

Conversation

Cohen-Carlisle
Copy link
Member

this commit adds 3 total tests

  • adds a non-pangram to resolve pangram: missing edge case #222
  • adds a pangram with underscores
  • adds a non-pangram where some letters have been replaced by numbers

the latter two are designed to detect solutions that rely on counting
the number of word characters (which include numbers and underscoe)

@kotp
Copy link
Member

kotp commented Apr 18, 2016

Missing the tests that states that numbers are allowed in a pangram. Such as "The quick brown fox jumped over the lazy dog for another 5 points." The number not invalidating the pangram. Neither does the capital T, nor the period.

The concern given before was 26 unique characters passing. That test seems to also be missing.

@Cohen-Carlisle
Copy link
Member Author

Missing the tests that states that numbers are allowed in a pangram. Such as "The quick brown fox jumped over the lazy dog for another 5 points." The number not invalidating the pangram. Neither does the capital T, nor the period.

We have one test case for pangram with mixed case and punctuation, as well as one for pangram with non ascii characters. Is there a case in which those pass but the one with numbers fails that we want to cover?

The concern given before was 26 unique characters passing. That test seems to also be missing.

I think you are referencing exercism/ruby#294, but its about not passing with 26 unique word characters, which the latter two tests address. Checking for 26 unique characters would fail at the quick brown fox jumps over the lazy dog as it has a space as well.

@kotp
Copy link
Member

kotp commented Apr 20, 2016

I guess @victorbrenders PR is not being used, so I guess it doesn't matter what his intention was. My response may have been in confusion with the now split concerns between two tests that may be meant to cover the same problem.

The test should be a pangram that has numbers, and it should ensure that it can pass. This is esting that if you are missing the 't' 'i' and 'o' it will fail. The numbers, again, are a distraction to what the test should be testing. We already have a test for missing letters. We don't have a test for pangram phrases that have numbers in them.

I think we should.

It also may be possible that a test should exist where you have `0123456789klmnopqrstuvwxyz' which is 26 unique (Regular Expression) word (class) characters. I think that @victorbrender was stating that there was a solution he had seen that would have had that pass based on a regular expression word class solution, and I think it was what his test was meant to test for. Though unsure, he has yet to confirm his intent.

@Cohen-Carlisle
Copy link
Member Author

I talked with @kotp and made some changes based on his feedback.

{
"description": "pangram with numbers",
"input": "the 1 quick brown fox jumps over the 2 lazy dogs",
"expected": false
Copy link
Member

Choose a reason for hiding this comment

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

should this be true, since it has all letters a-z? The presence of extra numbers should not cause the expected result to be false, right?

@Cohen-Carlisle
Copy link
Member Author

Ugh, bad copy paste job.

@Cohen-Carlisle
Copy link
Member Author

Cohen-Carlisle commented Apr 23, 2016

@petertseng updated my derp error. Thanks for catching that. I meant to copy over "the quick brown fox..." but copied "the quick brown fish..." by mistake. Corrected the "fish" part and forgot about the expectation 😿

this commit adds 4 total tests

1 non-pangram to resolve #222 (comparison to "abc...")
2 pangram with underscores
3 pangram with numbers
4 non-pangram where missing letters are replaced by numbers

2 and 4 are designed to fail solutions that rely on counting
the number of word characters (which include numbers and underscore)

3 is intended to clarify that numbers are allowed in a pangram
and thus their mere presence in 4 is not the cause of a failure
@Cohen-Carlisle
Copy link
Member Author

Squashed commits.

@kotp kotp merged commit 1cc803d into exercism:master Apr 23, 2016
@kotp
Copy link
Member

kotp commented Apr 23, 2016

@exercism/track-maintainers Don't forget to generate pangram tests for your track.

@Cohen-Carlisle Cohen-Carlisle deleted the add-pangram-cases branch May 17, 2016 21:37
emcoding pushed a commit that referenced this pull request Nov 19, 2018
add missing test to sum-of-multiples exercise
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.

pangram: missing edge case
3 participants